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

ci: make TestMailpit and TestProjectPortOverride more forgiving of Rancher Desktop, fixes #5578 #5594

Merged
merged 3 commits into from Dec 1, 2023

Conversation

rfay
Copy link
Member

@rfay rfay commented Dec 1, 2023

The Issue

TestMailpit fails intermittently on Rancher Desktop

This may be due to the mailpit API coming up after our healthcheck says it should be ready, because we're not polling the API.

How This PR Solves The Issue

Wait in the test a few seconds if necessary

It's possible this could also be fixed by adding more extensive healthcheck.sh to check the mailpit API, but that seems like overkill for something that is not used a huge amount.

@github-actions github-actions bot added the bugfix label Dec 1, 2023
Copy link

github-actions bot commented Dec 1, 2023

@rfay
Copy link
Member Author

rfay commented Dec 1, 2023

TestProjectPortOverride is also flaky on Rancher at this line, failed here: https://buildkite.com/ddev/macos-rancher-desktop/builds/120#018c22cd-9094-41f8-9452-60e03ce15bcd

assert.True(netutil.IsPortActive(app.RouterHTTPPort), "port "+app.RouterHTTPPort+" should be active")

@rfay rfay marked this pull request as ready for review December 1, 2023 17:47
@rfay rfay requested a review from a team as a code owner December 1, 2023 17:47
@rfay rfay changed the title fix: make TestMailpit more forgiving of Rancher Desktop, fixes #5578 ci: make TestMailpit more forgiving of Rancher Desktop, fixes #5578 Dec 1, 2023
@rfay
Copy link
Member Author

rfay commented Dec 1, 2023

This seems to be working OK for TestMailpit at this point.

However, TestProjectPortOverride often fails on rancher. It's possible there's something wrong with our test runner[s] and the port is still occupied somehow as well.

@rfay
Copy link
Member Author

rfay commented Dec 1, 2023

It looks like the healthcheck for traefik may also be checking not-exactly the right thing; it waits for the web server, but that's not the same as all the work being done. We may be able to improve the traefik healthcheck at some point, but 4e58ac2 just tries to give the test a little more time.

@rfay rfay changed the title ci: make TestMailpit more forgiving of Rancher Desktop, fixes #5578 ci: make TestMailpit and TestProjectPortOverride more forgiving of Rancher Desktop, fixes #5578 Dec 1, 2023
@rfay
Copy link
Member Author

rfay commented Dec 1, 2023

I think it worked!

Copy link
Member Author

@rfay rfay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is tests only, so I'm going to go ahead and pull it as it can stop the bleeding on some of the other PRs. We'll still have bleeding on Colima tests though.

@rfay
Copy link
Member Author

rfay commented Dec 1, 2023

If you have things you'd rather do with this @stasadev they're welcome as followup.

@rfay rfay merged commit c06e251 into ddev:master Dec 1, 2023
20 checks passed
@rfay rfay deleted the 20231130_rancher_wait_for_mailpit branch December 1, 2023 23:15
@stasadev
Copy link
Member

stasadev commented Dec 4, 2023

I think it is good enough for tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants