Skip op critical path benchmark test on FRS (all R11s actually)#25570
Skip op critical path benchmark test on FRS (all R11s actually)#25570markfields merged 4 commits intomicrosoft:mainfrom
Conversation
github-actions
bot
added
the
area: tests
There was a problem hiding this comment.
Pull Request Overview
This PR fixes flakiness issues with the op critical path benchmark test by excluding FRS (Fluid Relay Service) and R11s variants. Since the test intentionally ignores server interactions and doesn't measure op roundtrip time, the specific service used shouldn't affect the benchmark results.
- Adds conditional skipping for R11s and routerlicious driver types
- Changes arrow function to regular function to access
this.skip()
packages/test/test-end-to-end-tests/src/test/benchmark/opCriticalPath.time.spec.ts
Outdated
Show resolved
Hide resolved
markfields
force-pushed
the
op-bench-skip
branch
from
7dd6dc0 to
be218bd
Compare
|
Planning to hold merging this until #26018 goes in and I can see that it's working. Using the test failure this PR fixes to evaluate that one :) |
steffenloesch
left a comment
steffenloesch
left a comment
There was a problem hiding this comment.
Argument in the description makes sense. I'm OK with skipping.
| await setup(); | ||
| }, | ||
| benchmarkFnAsync: async () => { | ||
| // eslint-disable-next-line object-shorthand |
There was a problem hiding this comment.
Any particular reason for this change? Seems unnecessary.
There was a problem hiding this comment.
You might be right... but not going to revisit this, I've spent enough time on this change (and a related infra improvement)
packages/test/test-end-to-end-tests/src/test/benchmark/opCriticalPath.time.spec.ts
Outdated
Show resolved
Hide resolved
|
@steffenloesch care to approve again please? |
steffenloesch
left a comment
steffenloesch
left a comment
There was a problem hiding this comment.
Still looks good to me.
Description
Fixes AB#46428
This test intentionally ignores server interactions (specifically it doesn't measure the op roundtrip time), so it shouldn't matter which service it runs against. We have seen some flakiness against FRS so drop that variant.