-
Notifications
You must be signed in to change notification settings - Fork 553
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
base: main
Are you sure you want to change the base?
test(shared-tree): Remove the report output path to let the benchmark tool to post it to pipeline #25022
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 :/ There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
This @fluidframework-tree test step itself just goes over 1.5h |
||
}; | ||
|
||
module.exports = extendedConfig; |
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.
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.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.
Include it in the PR description