-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add tests pipeline #222
Merged
Merged
Add tests pipeline #222
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To align it with minSdkVersion in app/build.gradle.
After some experiments with "retry" test rules and "wait for view with ID" utility functions, I have decided I prefer "dumb" sleeps in the right places. It clearly identifies the problematic parts of the tests, and I believe this has benefits compared to simply re-trying any failing tests without knowing where the problem is.
Now that we have a test pipeline, that's the important check to look at it, and since it builds an APK, this job becomes superfluous.
This is needed for NotEventsTest.agenda_MultipleWithTimes(), among others.
MiscTest.fragmentTest() is insanely flaky, and it already has sleeps in many places since way back. Sometimes those sleeps just aren't enough, but increasing them would mean slowing everything down a lot. A retry rule is the only working solution I have found. NoteFragmentTest.testContentLineCountUpdatedOnNoteUpdate() is also just too damn flaky, and I cannot understand where the problem is. It just seems that, sometimes, onRecyclerViewItem() will not work, and waiting for the right ID does not help.
When a pull request is created from a fork, our repository secrets are not available to the workflow runner.
chvp
approved these changes
Apr 16, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
alensiljak
approved these changes
Apr 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have worked on this for quite some time in my fork. Those damn flaky Espresso tests...
I believe the tests are now stable enough. The job may still fail occasionally because of some infrastructure issue with GitHub, but it's usually pretty easy to find the cause in the output.
I decided to remove the "build" workflow, since I feel running the tests should be the main check to do when accepting contributions, and an APK is always built as part of that process.
I'm thinking the next step is to run tests on multiple API versions (lowest and highest supported), but when I tried some other API versions than 29, it seemed like some tweaking is needed before everything works properly. So I'm thinking this should be an acceptable first step.