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

[Prototype] Improve DDS stress testing: introduce enum stress mode #21702

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

clarenceli-msft
Copy link
Contributor

@clarenceli-msft clarenceli-msft commented Jun 28, 2024

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:

  1. 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.

  2. 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.

List any specific things you want to get reviewer opinions on, and anything a reviewer would need to know to review this PR effectively.
Things you might want to include:

  • Questions about how to properly make automated tests for your changes.
  • Questions about design choices you made.
  • Descriptions of how to manually test the changes (and how much of that you have done).
  • etc.

If you have any questions in this section, consider making the PR a draft until all questions have been resolved.

@clarenceli-msft clarenceli-msft requested review from a team as code owners June 28, 2024 20:36
@github-actions github-actions bot added area: build Build related issues area: dds Issues related to distributed data structures area: dds: sharedstring area: dds: tree area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch labels Jun 28, 2024
@msfluid-bot
Copy link
Collaborator

msfluid-bot commented Jun 28, 2024

@fluid-example/bundle-size-tests: +245 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 457.54 KB 457.58 KB +35 Bytes
azureClient.js 555.3 KB 555.35 KB +49 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 258.68 KB 258.7 KB +14 Bytes
fluidFramework.js 391.52 KB 391.53 KB +14 Bytes
loader.js 134.07 KB 134.08 KB +14 Bytes
map.js 42.12 KB 42.12 KB +7 Bytes
matrix.js 145.63 KB 145.63 KB +7 Bytes
odspClient.js 523.08 KB 523.13 KB +49 Bytes
odspDriver.js 97.17 KB 97.19 KB +21 Bytes
odspPrefetchSnapshot.js 42.27 KB 42.29 KB +14 Bytes
sharedString.js 162.64 KB 162.65 KB +7 Bytes
sharedTree.js 382.03 KB 382.04 KB +7 Bytes
Total Size 3.27 MB 3.27 MB +245 Bytes

Baseline commit: 957b194

Generated by 🚫 dangerJS against c6d935a

* 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
Copy link
Contributor

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)

Copy link
Contributor

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.

Copy link
Contributor Author

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.

@clarenceli-msft clarenceli-msft changed the title [Prototype] Improve DDS stress pipeline: add numeric stress level Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build Build related issues area: dds: sharedstring area: dds: tree area: dds Issues related to distributed data structures area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch
3 participants