-
Notifications
You must be signed in to change notification settings - Fork 523
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
[Prototype] Improve DDS stress testing: introduce enum stress mode #21702
base: main
Are you sure you want to change the base?
[Prototype] Improve DDS stress testing: introduce enum stress mode #21702
Conversation
⯅ @fluid-example/bundle-size-tests: +245 Bytes
Baseline commit: 957b194 |
* Packages which use this should generally have a `test:stress` script in their package.json for conveniently running | ||
* the equivalent checks locally. | ||
* It indicates the "stress level" of tests. A value of 0 means the test is not run in stress mode, while | ||
* higher values indicate increasingly stressful testing. This value also acts as a multiplier for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than make this numeric, I'd prefer we use an enum or string union with well-defined semantics for its members. Ideally a test author should be able to look at the docs here and make informed decisions on how to write their tests. The semantics should be in terms of the expectations for
- roughly how long should the tests take?
- are the tests expected to be evergreen? (true for deterministic seeds, false for random seeds)
in addition to other criteria you might find relevant. Because this doc is FF internal, I think it's fine if the documentation for those modes mentions the strategy we use for running tests in that mode (e.g. if we go with the strategies we discussed, we document here that a 'short' mode is run on all CI runs as part of the package's normal correctness tests, a 'medium' is run post check-in when a package is changed, and a 'long' is run on a fixed schedule using random seeds)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all the other changes in this PR look fine at a glance, I can review in more detail for places where there are missing changes of a similar sort (if any) and/or nitpick. but it generally looks like what I expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated the PR and included the unit tests for introducing enum stress modes. For some tests, like optionalChangeRebaser.test.ts
, parameters such as numberOfEditsToRebaseOver
should be customized by the area expert or owner, depending on the stress mode. Currently, I prefer to leave these parameters as they are, so they will only differ when stress mode is enabled as before.
…to add-stress-level-to-stress-tests
…to add-stress-level-to-stress-tests
AB#2729, AB#2730
This PR is a prototype for improving the Test - DDS Stress line by using a numeric stressLevel instead of a boolean isStress. This change allows for more granular control over whether to use stress runs and to what degree.
The follow-up tasks include:
Integrating this functionality (potentially as a stage) into the CI build pipeline with a light stress level mode, so it can be triggered in the PR gateway.
Creating a long-running pipeline to execute the DDS stress tests with a heavy stress level mode, scheduled to run periodically during off-peak hours, similar to the test stability pipeline.
Guidelines for Pull Requests.
Reviewer Guidance
The review process is outlined on this wiki page.