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

Turn Off Flaky C/I Integration Tests & Design Plan to Improve/Replace Them #2326

Closed
danforbes opened this issue Feb 22, 2022 · 2 comments
Closed
Assignees

Comments

@danforbes
Copy link
Contributor

We have some C/I tasks that are flaky (e.g. docker-rpc and docker-stress, I think). These tests oftentimes fail because the test infrastructure itself is not robust and deterministic - the tests are not failing because there are bugs in actual Gossamer production code, necessarily. This is bad because it wastes developer time, and also we can have very little confidence that these tests are actually checking anything meaningful if the test infrastructure itself is not correct.

This Issue can be closed when there is a plan in place to improve/replace the flaky tests (an Issue(s) should be created to track this) and the flaky tests have been turned off in C/I.

@qdm12
Copy link
Contributor

qdm12 commented Mar 11, 2022

The following comment will be updated as I progress

A few simple refactoring that looks needed for now:

  • Timeouts are a defined in each function locally, making it hard to define timeouts from the actual test. We should instead use contexts and injected timeout values (as arguments) for operations.
  • All operations should have retry mechanisms retrying until the context is canceled or times out
  • RPC operations should have a client function for each 'method', instead of passing method constant strings

More refactoring that can be done:

  • Run gossamer using go run instead of pre-building it and then running it. That shouldn't impact performance since Go will not rebuild it on the second time, and should remove the need for pre-tests setups
  • dependency inject loggers (chore(maintainability): inject logger instead of using global loggers #1946) so that logs can be inspected from the tests using a io.Writer interface such as as a buffer.

@qdm12
Copy link
Contributor

qdm12 commented Mar 17, 2022

Issues were created and reference this issue.

Since it was rather hard to plan ahead how to fix the messy-ish test code, work for them is mostly done already.

The issues created address refactoring of the test code, and this should allow us to have more robust tests with easily adjustable timeout values.

Closing this since there is a plan in the forms of issues. More issues may be created later if it turns out the refactoring + timeout adjustments is not sufficient.

@qdm12 qdm12 closed this as completed Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants