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

support using failpoints from unit tests without requiring serial test execution #79

Open
rodesai opened this issue Jul 4, 2024 · 5 comments

Comments

@rodesai
Copy link

rodesai commented Jul 4, 2024

Is your feature request related to a problem? Please describe.
Currently failpoints cannot be used from tests without requiring all the tests that hit failpoints to be executed serially. This essentially prevents us from using failpoints from our unit tests without forcing tests to run with a single testing thread. The suggested approach is to place failpoint tests under the tests tree so that they are executed as rust integration tests. However, this means that the tests cannot use any interfaces that are not exposed by the crate, which makes it difficult to write most of our test cases.

Describe the solution you'd like
The reason for this restriction is that failpoints uses a global failpoint registry to control fault injections. It would be nice if there were a way to set up failpoints so that they could use a test-specific registry. One approach to doing this is to support specifying the failpoint registry in calls to the fail_point! macros, e.g.

let registry = <construct or accept a passed in failpoint registry>
fail_point!(&registry, "fail-a-thing", |_| std::io::Error::new(...))

Describe alternatives you've considered
We considered running tests serially and moving our fault tests to tests. Running tests serially might work for now, but could lead to longer build times later. The bigger problem is that it causes tests to fail by default, so developers in our project would always need to remember to run tests with a single thread and configure their ide to do the same, which is painful. Putting tests in tests is not ideal because it requires us to expose a lot of interfaces from our crate that we don't want to to write the tests we want to write.

@BusyJay
Copy link
Member

BusyJay commented Jul 5, 2024

See also #51. In TiKV, we solve this by splitting test cases into different groups and launch the multiple process to run those test group concurrently. Test case in the same test group is run sequentially.

@criccomini
Copy link

@BusyJay I see #51 is still open. Two questions:

  1. Any docs or pointers on how to split into groups?
  2. Will this work for module unit tests in the same .rs file, not just integration tests in tests?

The reason I ask (2) is because we have some APIs we want to test but not expose publicly.

@BusyJay
Copy link
Member

BusyJay commented Jul 6, 2024

Any docs or pointers on how to split into groups?

No, it's just some scripts.

Will this work for module unit tests in the same .rs file, not just integration tests in tests?

Yes. We used to manually collecting built test binaries and list all available cases. Now we use cargo nextest to make the process more maintainable. You can find all the steps in https://do.pingcap.net/jenkins/blue/organizations/jenkins/tikv%2Ftikv%2Fpull_unit_test/detail/pull_unit_test/995/pipeline/71, build step and test step specifically.

@criccomini
Copy link

Any interest in taking a PR that allows users to pass a failpoint registry in rather than use the global?

@criccomini
Copy link

Here is our forked repo, for reference:

And here's the actual commit that adds registry parameter support:

Now users may supply a registry when the call various fail_point/cfg/etc methods. Doing so will use the supplied registry instead of the static REGISTRY.

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

3 participants