Remove state from EventQueueTests and other test refinements
#78
Conversation
This file contains hidden or 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
mokagio
commented
Apr 20, 2023
Comment on lines
+12
to
22
| func makePareslyTracker() -> Parsely { | ||
| let tracker = Parsely.getInstance() | ||
|
|
||
| // Note that because we call addTeardownBlock, this method needs to be defined within an | ||
| // XCTestCase | ||
| addTeardownBlock { | ||
| tracker.hardShutdown() | ||
| } | ||
|
|
||
| return tracker | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Using addTeardownBlock in the context of makeParselyTracker() makes sure that any test calling this method will call hardShutdown() on completion. This gives us the ability to call the method and assign its return value to an instance scoped at the test method without the risk of it remaining in memory after the test finishes.
mokagio
changed the title
EventQueueTests and other test refinements
Contributor
The first GitHub Action run failed at a flaky test which should be fixed in #79 |
It did not need access to the `ParselyTestCase` APIs
It did not need access to the `ParselyTestCase` APIs
It did not need access to the `ParselyTestCase` APIs.
It did not need access to the `ParselyTestCase` APIs.
The next step will be to remove the setup method and let subclasses call `makeParselyTracker()`.
mokagio
force-pushed
the
mokagio/test-without-shared-instance
branch
from
4c09a85 to
ec6b4bd
Compare
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.
XCTestCasesubclasses instead ofParselyTestCase. They didn't need access to the tracker test instanceEventQueueTestsandEngagedTimeTests. Now each test methods create objects whose lifecycle ends together with the test. They will not survive in memory after the test finished.ParselyTestCaseto offer a single API to generate aParselyinstance for tests configured to callhardShutdown()when the test finishes.My editor removed whitespaces from the files. I recommend hiding them when reviewing to reduce the noise. Also, commit by commit review might be clearer.
CI runs only on PRs against
master. I obviously run the tests locally and verified everything still works but feel free to doublecheck 😄