-
Notifications
You must be signed in to change notification settings - Fork 373
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
CI - Fix failing test 2024-02-14; Includes fixes for RobolectricTest, updating to Android compile API 34, and fixing a variety of tests #1994
Conversation
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.
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.
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
75d7664
to
900d76a
Compare
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having this here doesn't affect SDK consumers?
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting this column was added in #1793
Description
One Line Summary
Fix failing tests and test improvements; including fixing silently skipped tests due to
RobolectricTest
issues and android versions.Details
The main focus of the PR was to fix unit tests using
@RobolectricTest
being skipped and fixing any failing tests. This was done by making changes toRobolectricExtension
. Some other changes are:RobolectricExtension
by adding a newtesthelpers
module 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