Open
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
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.
Improving the robustness of the mock api and with that the reliability of the e2e tests.
Added caching for the mock image
I've bumped into the occassional 502 error when fetching the php5.6 image from docker.
There was no healthcheck on the image so the process would just continue and the e2e failed because the mock api was not available. Every push will run 7 versions of the integration test and rebuild the same image 7 times over while nothing had changed. Caching the image would speed up the tests by 20s~ and reduce the chance of running into the 502 hiccup.
Before:


After:
(additional reading: https://www.blacksmith.sh/blog/cache-is-king-a-guide-for-docker-layer-caching-in-github-actions)
Added wait and exit on error
Caches can expire or get evicted so the 502 hiccup can still happen. If that happens we want to know before running the integration tests. I've added a health-check on the docker compose for the mock image and will wait for a succesful exit, otherwise exit on error.
Remove e2e for S3 Offload
Having full e2e tests is nice but this had a lot of dependencies:
I could cache localstack as well but all felt quite a lot. In the end the user base for the plug-in is not high enough to justify the cost of having these dependencies.