Skip to content

Comments

bullet proof smtp (fix flaky test)#2868

Merged
supersven merged 47 commits intodevelopfrom
sventennie/bullet_proof_smtp
Dec 22, 2022
Merged

bullet proof smtp (fix flaky test)#2868
supersven merged 47 commits intodevelopfrom
sventennie/bullet_proof_smtp

Conversation

@supersven
Copy link
Contributor

@supersven supersven commented Nov 24, 2022

This is almost the same as #2818, but should fix the test instability by using a different way to create random ports for the test SMTP server (postie).

The solution is inspired by our MockServer.

The approach can be sub-summed as:

  • Use bindRandomPortTCP of streaming-commons. I.e. don't deal with all the traps and pitfalls of primitive Socket handling; let the library do it.
  • Once the Socket is open, use it (instead of the port number). This should prevent race-conditions on the port.

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@supersven supersven temporarily deployed to cachix November 24, 2022 15:52 Inactive
@supersven supersven temporarily deployed to cachix November 24, 2022 15:52 Inactive
@supersven supersven force-pushed the sventennie/bullet_proof_smtp branch from 869a934 to c1e4d5e Compare November 24, 2022 16:55
@supersven supersven temporarily deployed to cachix November 24, 2022 16:55 Inactive
@supersven supersven temporarily deployed to cachix November 24, 2022 16:55 Inactive
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Nov 24, 2022
@supersven supersven temporarily deployed to cachix November 25, 2022 09:58 Inactive
@supersven supersven temporarily deployed to cachix November 25, 2022 09:58 Inactive
@supersven supersven temporarily deployed to cachix November 25, 2022 12:22 Inactive
@supersven supersven temporarily deployed to cachix November 25, 2022 12:22 Inactive
@supersven supersven temporarily deployed to cachix November 28, 2022 06:35 Inactive
@supersven supersven temporarily deployed to cachix November 28, 2022 06:35 Inactive
@supersven supersven marked this pull request as ready for review November 28, 2022 07:25
@supersven supersven force-pushed the sventennie/bullet_proof_smtp branch from d186de2 to b319a00 Compare December 14, 2022 08:31
@supersven supersven temporarily deployed to cachix December 14, 2022 08:31 — with GitHub Actions Inactive
@supersven supersven temporarily deployed to cachix December 14, 2022 08:31 — with GitHub Actions Inactive
supersven and others added 5 commits December 21, 2022 08:30
Co-authored-by: fisx <mf@zerobuzz.net>
Use random sockets (and by this ports) generated by a library. Keep the
sockets open while the test runs. (Preventing further race-conditions.)
We don't want to run out of file handles...
@supersven supersven force-pushed the sventennie/bullet_proof_smtp branch from b319a00 to 75446ea Compare December 21, 2022 07:30
@supersven supersven temporarily deployed to cachix December 21, 2022 07:30 — with GitHub Actions Inactive
@supersven supersven temporarily deployed to cachix December 21, 2022 07:30 — with GitHub Actions Inactive
@supersven supersven temporarily deployed to cachix December 21, 2022 07:52 — with GitHub Actions Inactive
@supersven supersven temporarily deployed to cachix December 21, 2022 07:52 — with GitHub Actions Inactive
@supersven supersven temporarily deployed to cachix December 21, 2022 07:54 — with GitHub Actions Inactive
@supersven supersven temporarily deployed to cachix December 21, 2022 07:54 — with GitHub Actions Inactive
Copy link
Contributor

@battermann battermann left a comment

Choose a reason for hiding this comment

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

LGTM, only nitpicks, feel free to ignore :)

Fix grammar

Co-authored-by: Leif Battermann <leif.battermann@wire.com>
@supersven supersven temporarily deployed to cachix December 21, 2022 09:40 — with GitHub Actions Inactive
@supersven supersven temporarily deployed to cachix December 21, 2022 09:40 — with GitHub Actions Inactive
Code improvements

Co-authored-by: Leif Battermann <leif.battermann@wire.com>
@supersven supersven temporarily deployed to cachix December 21, 2022 09:42 — with GitHub Actions Inactive
@supersven supersven temporarily deployed to cachix December 21, 2022 09:42 — with GitHub Actions Inactive
@supersven
Copy link
Contributor Author

LGTM, only nitpicks, feel free to ignore :)

@battermann , all "nitpicks" were considered useful. Thanks a lot for them!

@supersven supersven temporarily deployed to cachix December 21, 2022 10:58 — with GitHub Actions Inactive
@supersven supersven temporarily deployed to cachix December 21, 2022 10:58 — with GitHub Actions Inactive
@supersven supersven temporarily deployed to cachix December 21, 2022 13:03 — with GitHub Actions Inactive
@supersven supersven temporarily deployed to cachix December 21, 2022 13:03 — with GitHub Actions Inactive
@supersven supersven merged commit b647ecc into develop Dec 22, 2022
@supersven supersven deleted the sventennie/bullet_proof_smtp branch December 22, 2022 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants