Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

fix(ci): flaky tests #619

Merged
merged 1 commit into from Jun 22, 2023
Merged

fix(ci): flaky tests #619

merged 1 commit into from Jun 22, 2023

Conversation

vados-cosmonic
Copy link
Contributor

Feature or Problem

This test fixes some tests that were flaky in CI and removes waits.

Tests have only become slightly faster (on the order of seconds).

Some additional fixes were made to the nix tests, but it's not clear why they're failing (now that all deps are present, it runs for 25 minutes...)

Related Issues

Release Information

next

Consumer Impact

Developers can more readily trust failing test runs to mean actual regressions.

Testing

Built on platform(s)

  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
  • x86_64-windows

Tested on platform(s)

  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
  • x86_64-windows

Unit Test(s)

Acceptance or Integration

Manual Verification

Cargo.toml Outdated Show resolved Hide resolved
connorsmith256
connorsmith256 previously approved these changes Jun 21, 2023
Copy link
Contributor

@connorsmith256 connorsmith256 left a comment

Choose a reason for hiding this comment

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

LGTM! One item for consideration: it looks like we mostly call wait_until_process_has_count on beam.smp with a predicate where we check for a count of 0. In one case we check for nats-server with a count of 1. If these are the only reasonable use cases, I'd consider wrapping these blocks in functions like wait_for_host_to_stop and wait_for_nats_to_start. Then you could remove the comments on these blocks, since the function call is self-describing :)

@vados-cosmonic
Copy link
Contributor Author

Hey @connorsmith256 yeah that's reasonable, will add a helper function that wraps that re-used functionality at a higher level.

One thing I don't want to do is to make it seem like we're actually waiting for the host in an intelligent way though -- might make people think we're not just grepping for processes (essentially), since we don't know the PIDs to wait on specifically necessarily.

While I'm in here I'm going to mark the macOS nix build as optional since sometimes it OOMs (and there's not much we can do about that right now)

connorsmith256
connorsmith256 previously approved these changes Jun 21, 2023
@vados-cosmonic
Copy link
Contributor Author

vados-cosmonic commented Jun 21, 2023

One more rebase (I refactored and made the nix MacOS builds optional for now) -- @connorsmith256 if you wouldn't mind taking another look (and possibly merging!)

@vados-cosmonic
Copy link
Contributor Author

vados-cosmonic commented Jun 22, 2023

Squashed once more, fixed the release check.

To make it clear here, I'm expecting release branches like this one to contain release somewhere (ex. release/v0.18.0) -- if this isn't standardized/process it won't work

Signed-off-by: Victor Adossi <vadossi@cosmonic.com>
@connorsmith256 connorsmith256 merged commit eb9de36 into wasmCloud:main Jun 22, 2023
20 checks passed
@vados-cosmonic vados-cosmonic deleted the fix/flaky-tests branch July 6, 2023 12:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants