CI - Fix failing test 2024-02-14; Includes fixes for RobolectricTest, updating to Android compile API 34, and fixing a variety of tests #1994
+276
−769
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
jkasten2
force-pushed
the
ci/fix-unit-tests-2024-02-14
branch
2 times, most recently
from
ec15313
to
174a7ca
Compare
This fixes an error compile tests for in-app messages. androidx.work:work-runtime-ktx:2.8.1 was interduced and it requires android compile SDK version 33.
Replaced ShadowFusedLocationProviderApi with
FusedLocationApiWrapperMock as it depended on
Field::class.java.getDeclaredField("modifiers") which was a reflection
feature dropped in somewhere between Java 11 -> 17.
Since there wasn't a straightforward way to keep this as a shadow it
was worth the time instead to use dependency inject.
PR #1900 intorduced FAIL_PAUSE_OPREPO but never updated the test
PR #1794 made changes to omit aliases on user create, but the test was never updated. Also the SubscriptionStatus arg part of this test wasn't compare the corret types, causing it to fail the test.
It seems Kotest's intercept caller eats expections and moves on silently. Adding a try-catch and reporting TestResult.Error to solve this.
The first class with @RobolectricTest would run fine, but then every test class after in the run would fail with a looper already started error. This fix was discovered by looking through Robolectric's RobolectricTestRunner and SandboxTestRunner clases. Most of the pre-existing kotest-extensions-robolectric code seemed to be reripped from those 2 classes and I notice this was something this extension was not doing. Looper's are linked to a thread so this seems related, however I don't understand any more than this to explain why this fixes the error.
De-duplicated ContainedRobolectricRunner.kt and RobolectricExtension.kt by moving them into their own module. Before landing on this implementation I attempted to use the gradle feature testFixtures, however this doesn't work with the android gradle plugin.
jkasten2
force-pushed
the
ci/fix-unit-tests-2024-02-14
branch
from
9b16638
to
b7ff4b7
Compare
This makes sure you can see all failures
Test were being run 3 times since it was running debug, release, and unity targets.
jkasten2
changed the title
jkasten2
changed the title
jkasten2
requested review from
brismithers,
jennantilla,
nan-li and
emawby
and removed request for
brismithers
jkasten2
changed the title
In a previous commit I assumed all 4 copies of RobolectricExtension.kt were the same, however there was a difference in their getConfig implementation. This fixes a number of failing tests, such as the location tests.
id isn't a valid parameter to the OneSignal POST /subscriptions REST API
Test was out-of-date to the production changes we made a while ago.
This was the only test failing in this file and now it is passing.
Update allways follow from the module to resulting in making a network request to reflect it on the backed. An update operation will never result in property update to the model. This was proabably true at one point the code base, but if so it was a mistake.
If we get a 404 when we try to delete then it was already done at some point, so we count it as successful
jkasten2
force-pushed
the
ci/fix-unit-tests-2024-02-14
branch
from
75d7664
to
900d76a
Compare
jkasten2
changed the title
nan-li
approved these changes
Feb 27, 2024
| @@ -25,3 +25,4 @@ include ':OneSignal:core' | |||
| include ':OneSignal:in-app-messages' | |||
| include ':OneSignal:location' | |||
| include ':OneSignal:notifications' | |||
| include ':OneSignal:testhelpers' | |||
There was a problem hiding this comment.
Having this here doesn't affect SDK consumers?
There was a problem hiding this comment.
Right, the include in the setting.gradle is just to define projects are available to the workspace:
https://www.baeldung.com/gradle-build-settings-properties#settings-gradle
| @@ -171,13 +171,15 @@ class OutcomeEventsRepositoryTests : FunSpec({ | |||
| OutcomeEventsTable.COLUMN_NAME_NAME to "outcomeId1", | |||
| OutcomeEventsTable.COLUMN_NAME_WEIGHT to 0.2f, | |||
| OutcomeEventsTable.COLUMN_NAME_TIMESTAMP to 1111L, | |||
| OutcomeEventsTable.COLUMN_NAME_SESSION_TIME to 1L, | |||
There was a problem hiding this comment.
Noting this column was added in #1793
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.
Description
One Line Summary
Fix failing tests and test improvements; including fixing silently skipped tests due to
RobolectricTestissues and android versions.Details
The main focus of the PR was to fix unit tests using
@RobolectricTestbeing skipped and fixing any failing tests. This was done by making changes toRobolectricExtension. Some other changes are:RobolectricExtensionby adding a newtesthelpersmodule to share this classMotivation
It's important to keep our CI test works and up to date to catch bugs early.
Scope
Only fixing testing related issues.
Reviewing
Recommend reviewing commit-by-commit
Testing
Unit testing
All Unit Tests are now passing
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is