Closed skwashd closed 2 years ago
Hi @skwashd, thanks for this PR. We took your suggestions into account and fixed the integration tests during our PR last week to support arm64 architecture ( #52 ). Your PR also helped us realize that we had lost the latest tests (that had the sleep
in the wrong order and you fixed), and we included your fix when bringing back those tests as well.
So, I'll close this PR since it's no longer relevant, but we'll review the other ones that you sent. Sorry for the delay.
@valerena thanks for the update. It's good to know that it was useful even if it didn't get committed. Related to this, it would be good to see GitHub Actions or something else running tests so problems like this aren't missed in the future.
@skwashd For sure. We've changed the maintainer team a couple of times, so we're still working on making it better. Thanks for the suggestion.
Issue #, if available:
47
Description of changes: The integration tests were failing due to multiple issues.
Name conflicts
Multiple tests were using the same container name (
testing
), which meant some of the tests weren't running. Now each container has a unique name.Port conflicts
The same port (9000) was being used for multiple tests. This wasn't showing as an issue as these tests also used the same container name. The ports are now unique per test, with them being number sequentially to avoid any conflicts.
Teardown properly
Only some of the running containers were terminated when the suite finished executing. This caused many tests to not run properly on subsequent invocation of the suite. Now all containers are terminated when the tests complete.
Wrong order of operations
The timeout tests weren't waiitng for the container be ready. This was caused by the order of operations being wrong, with the sleep step occurring after the request to the endpoint, not before.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.