Skip to content

test(shared-tree): Remove the report output path to let the benchmark tool to post it to pipeline #25022

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/dds/tree/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
"test:customBenchmarks": "mocha --config ./.mocharc.customBenchmarks.cjs",
"test:customBenchmarks:verbose": "cross-env FLUID_TEST_VERBOSE=1 npm run test:customBenchmarks",
"test:memory": "mocha --perfMode --config ./src/test/memory/.mocharc.cjs",
"test:memory-profiling:report": "mocha --config ./src/test/memory/.mocharc.cjs",
"test:memory-profiling:report": "mocha --perfMode --config ./src/test/memory/.mocharc.cjs",
Copy link
Contributor

Choose a reason for hiding this comment

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

For reference: benchmarks always need the --perfMode flag to execute as such (and not as correctness unit tests). This missing one was an oversight from the past.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Include it in the PR description

"test:mocha": "npm run test:mocha:esm && echo skipping cjs to avoid overhead - npm run test:mocha:cjs",
"test:mocha:cjs": "cross-env MOCHA_SPEC=dist/test mocha",
"test:mocha:esm": "mocha",
Expand Down
1 change: 1 addition & 0 deletions packages/dds/tree/src/test/memory/.mocharc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const extendedConfig = {
"node-option": [...baseNodeOptions, "expose-gc", "gc-global", "unhandled-rejections=strict"], // without leading "--"
"reporter": "@fluid-tools/benchmark/dist/MochaReporter.js",
"reporterOptions": ["reportDir=.memoryTestsOutput/"],
"timeout": "90000",
Copy link
Contributor

Choose a reason for hiding this comment

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

@chentong7 I believe you confirmed we can use a much lower timeout, right?

Copy link
Contributor Author

@chentong7 chentong7 Jul 18, 2025

Choose a reason for hiding this comment

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

And it timed out at the end. Instead of making a flaky test, slow test is more acceptable.
Error: Timeout of 80000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

Copy link
Contributor

Choose a reason for hiding this comment

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

80000 or 8000? If we have individual tests that are really taking 80s when run as benchmarks, we might need to rethink our strategy for them :/

Copy link
Contributor Author

@chentong7 chentong7 Jul 18, 2025

Choose a reason for hiding this comment

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

Actually, it's worse than 80s. Each test case goes 30s makes the total test set to 90s, and some old test cases just running forever.

Overall summary

status  suite name                                                                          # of passed tests  total time (s)
------  ----------------------------------------------------------------------------------  -----------------  --------------
    ✔   SharedTree table APIs memory usage Column Insertion                                        3 out of 3            90.1
    ✔   SharedTree table APIs memory usage Row Insertion                                           3 out of 3            90.1
    ✔   SharedTree table APIs memory usage Column and Row Insertion                                2 out of 2            60.1
    ✔   SharedTree table APIs memory usage Column Insertion                                        3 out of 3            90.1
    ✔   SharedTree table APIs memory usage Row Insertion                                           3 out of 3            85.2
    ✔   SharedTree table APIs memory usage Column and Row Insertion                                2 out of 2            38.9
    ✔   SharedTree table APIs memory usage Column Removal                                          2 out of 2            60.1
    ✔   SharedTree table APIs memory usage Row Removal                                             3 out of 3            82.8
    ✔   SharedTree table APIs memory usage Column and Row Removal                                  2 out of 2            60.1
    ✔   SharedTree table APIs memory usage Insert a column and a row and remove right away         1 out of 1            30.1
    ✔   SharedTree table APIs memory usage Cell Value Setting                                      3 out of 3            89.0
    ✔   SharedTree table APIs memory usage Column Insertion                                        3 out of 3            71.9
    ✔   SharedTree table APIs memory usage Row Insertion                                           3 out of 3            68.8
    ✔   SharedTree table APIs memory usage Column and Row Insertion                                2 out of 2            41.2
    ✔   SharedTree table APIs memory usage Column Insertion                                        3 out of 3            70.7
    ✔   SharedTree table APIs memory usage Row Insertion                                           3 out of 3            70.8
    ✔   SharedTree table APIs memory usage Column and Row Insertion                                2 out of 2            39.8
    ✔   SharedTree table APIs memory usage Column Removal                                          2 out of 2            45.1
    ✔   SharedTree table APIs memory usage Row Removal                                             3 out of 3            73.2
    ✔   SharedTree table APIs memory usage Column and Row Removal                                  2 out of 2            47.9
    ✔   SharedTree table APIs memory usage Insert a column and a row and remove right away         1 out of 1            16.6
    ✔   SharedTree table APIs memory usage Cell Value Setting                                      3 out of 3            68.4
    ✔   SharedTree table APIs memory usage Column Removal                                          2 out of 2            43.0
    ✔   SharedTree table APIs memory usage Row Removal                                             3 out of 3            70.8
    ✔   SharedTree table APIs memory usage Column and Row Removal                                  2 out of 2            47.1
    ✔   SharedTree table APIs memory usage Insert a column and a row and remove right away         1 out of 1            20.9
    ✔   SharedTree table APIs memory usage Cell Value Setting                                      3 out of 3            69.5
    ✔   SharedTree memory usage Forest memory usage Array of monomorphic leaves                    8 out of 8           840.6
    ✔   SharedTree memory usage Forest memory usage Array of polymorphic leaves                    8 out of 8           837.8
    ✔   SharedTree memory usage Forest memory usage Array of deep monomorphic leaves               8 out of 8         1,016.4
    !   SharedTree memory usage                                                                   4 out of 10           120.8
        total                                                                                    93 out of 99         4,458.2

This @fluidframework-tree test step itself just goes over 1.5h

};

module.exports = extendedConfig;
1 change: 1 addition & 0 deletions tools/pipelines/test-perf-benchmarks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ parameters:
- name: memoryTestPackages
type: object
default:
- "@fluidframework/tree"
- "@fluidframework/sequence"
- "@fluidframework/map"
- "@fluidframework/matrix"
Expand Down
Loading