Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

try to make the excluded bug appear in CI #159

Merged
merged 5 commits into from
Jul 8, 2022
Merged

Conversation

kali
Copy link
Collaborator

@kali kali commented Jul 6, 2022

No description provided.

Comment on lines +67 to +70
$ANDROID_SDK_ROOT/cmdline-tools/latest/bin/sdkmanager --install "system-images;android-24;default;armeabi-v7a" "ndk;22.1.7171670"
echo no | $ANDROID_SDK_ROOT/cmdline-tools/latest/bin/avdmanager create avd -n testdinghy -k "system-images;android-24;default;armeabi-v7a"
$ANDROID_SDK_ROOT/emulator/emulator @testdinghy -no-audio -no-boot-anim -no-window -accel on -gpu off &
timeout 180 $ANDROID_SDK_ROOT/platform-tools/adb wait-for-device
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on using a github action for this? ReactiveCircus/android-emulator-runner looks like it might do the trick.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem with this github action is that it only works on macos runners, I wanted to have something that was able to test android on linux ^^ plus having those 4 lines here in the script means it is a bit easier to replicate when testing locally

Copy link
Contributor

@simlay simlay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hope you don't mind me doing a drive by review.

Anyway, given how much this is modifying .travis.sh, it might make sense to break the script into two or three scripts. Something like .github/ios.sh and .github/android.sh and .github/host.sh where the host.sh would be running dinghy on the host machines.

Admittedly, it seems that this PR is to fix some bug that's appearing outside CI.

@fredszaq fredszaq force-pushed the fix-excluded-tests branch 3 times, most recently from 7e6d109 to ad8c185 Compare July 7, 2022 13:39
@fredszaq fredszaq force-pushed the fix-excluded-tests branch from ad8c185 to 8a9ec62 Compare July 7, 2022 13:47
@fredszaq
Copy link
Collaborator

fredszaq commented Jul 7, 2022

This PR fixes the few problems we have found when migrating the tract ci to the new version

  • the script device was broken (we added one to the ci using qemu to make sure it doesn't happen again)
  • the path of the executable was passed as the first argument to the executables by the runner which led to unwanted filtering of when running tests
  • the resource mapping file for the script device was not generated at the right place meaning resources were not available in for script devices

The ci script could indeed be split by device, especially as we should add tests for the ssh device which is not tested in the ci right now (we can probably come up with something using docker) not sure we'll do that in this pr, as the goal here is to fix the broken 0.5.0 release

@fredszaq fredszaq merged commit 30811b1 into main Jul 8, 2022
@fredszaq fredszaq deleted the fix-excluded-tests branch July 8, 2022 09:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants