Skip to content
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

Fix Retrofit adapter test configuration #2624

Merged
merged 3 commits into from
Jan 11, 2022

Conversation

lukaszkalnik
Copy link
Contributor

@lukaszkalnik lukaszkalnik commented Jan 4, 2022

Current test configuration reuses one MockWebServer instance across all tests. This is incorrect, as any changes to the mock server configuration influence subsequent tests.
To experience the problem on the current main branch, change the test order in that you move any test asserting a successful response, e.g. should return ResponseMock for 200 with valid JSON, after the last test should throw when server disconnects. The test for a successful response will now start failing.

This PR fixes that in that the server and service variables have been made lateinit vars and are initialized before any test.
To test if this fixes the problem, just change the test order in the way described above on this PR's branch and see that the tests all still pass regardless of their order.

I have also renamed the confusingly named SuspedApiClientTest to SuspendApiTestClient. The old name suggested that it was a test (because of the Test suffix) when in reality it is a test client.

@lukaszkalnik
Copy link
Contributor Author

@nomisRev the build is failing with the following error:

* What went wrong:
A problem occurred configuring project ':arrow-optics-test'.
> java.util.concurrent.TimeoutException

@nomisRev
Copy link
Member

nomisRev commented Jan 7, 2022

This should be solved now @lukaszkalnik if you merge main.

@nomisRev nomisRev merged commit 4306588 into arrow-kt:main Jan 11, 2022
@lukaszkalnik lukaszkalnik deleted the fix-retrofit-adapter-test-config branch January 11, 2022 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants