-
Notifications
You must be signed in to change notification settings - Fork 549
improvement(test-version-utils): Update cross-client compat matrix #24815
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?
Conversation
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.
Pull Request Overview
This PR updates the cross-client compatibility matrix to include both N-1 and N-2 versions (for legacy+alpha minor and public major releases) in addition to existing LTS versions, filtering out duplicates and versions below 1.0.0.
- Introduces
currentCrossClientVersionDeltas
to the default matrix - Refactors
genCrossClientCompatConfig
to builddeltaVersions
in three categories (minor, major, LTS) and filter them - Preserves test order (minor → major → LTS) and avoids duplicate or unsupported versions
Comments suppressed due to low confidence (2)
packages/test/test-version-utils/src/compatConfig.ts:304
- [nitpick] The doc comment mixes
*
and-
bullet styles. Consider standardizing to one style (e.g., all* -
) and aligning indentation for clarity.
* - N-1 and N-2, for legacy+alpha breaking minor releases (i.e. \>=2.10.0 \<2.20.0, \>=2.20.0 \<2.30.0, etc.)
packages/test/test-version-utils/src/compatConfig.ts:366
- The new N-2 inclusion and duplicate/filter logic in
genCrossClientCompatConfig
is non-trivial. Please add unit tests to verify that the output includes N-1, N-2, and LTS combos and excludes duplicates or versions <1.0.0.
return configs;
Haven't reviewed the changes yet, but nit from the description + screenshot: we say that the N-1 public version (1.4.0) is included, then the LTS N-1 is excluded because it's the same one, but the output has a line for |
@alexvy86, updated in latest! See the updated PR description for the new example output |
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 don't see anything obviously concerning. And I see why my offline suggestion to move the "compat kind" close to the beginning of the string instead of having it in the deltas could be tricky. I think I still like it better but not blocking on it.
I'll bring my other suggestion here for visibility: I still think it reads slightly better to swap the actual versions and the delta specifiers in/out of parenthesis, i.e. create with N (2.43.0) + load with N-1 legacy+alpha (2.33.2)
instead of create with 2.43.0 (N) + load with 2.33.2 (N-1 legacy+alpha)
, but very minor and again not blocking.
Also disclaimer: my brain is a bit fried at this point today so probably get another pair of eyes on this :)
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.
Let's not use legacy+alpha
in any of these places. We need a more appropriate term. It is not legacy+alpha
that drives things nor are the compat needs limited to legacy+alpha
tagged items. legacy+alpha
is some tagging that we are applying to a set of APIs and we happen to allow breaking them at certain times.
We are about to stop using alpha
tag for this API surface. Eventually we'd like to avoid legacy
tag too and have things be public
. But that will not change the compat requirements.
Description
This PR updates our cross-client compat generation matrix to include N-1 and N-2. This is done to align with our new cross-client compat policy.
Updated Matrix
With these changes, we test the current version (N) against the following releases:
The following is an example output for a test using
"FullCompat"
with these changes. Please note:-LTS = Merged with public major release N-1 since it is a duplicate version (1.4.0)
Misc
AB#38596