-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
doc: add fullName property to SuiteContext #60762
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?
doc: add fullName property to SuiteContext #60762
Conversation
|
Review requested:
|
Ethan-Arrowood
left a comment
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'm just not sure about the version number otherwise lgtm
Renegade334
left a comment
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.
LGTM but please address the commit linter error.
Add documentation for the missing fullName property in SuiteContext class. This property returns the name of the suite and each of its ancestors, separated by '>'. The property has been available since v22.3.0 but was missing from the documentation. Fixes: nodejs#60757
43e96d5 to
4a9adfe
Compare
|
Thanks for the reviews! You're right about the version - I checked PR #53169 and the feature was backported to v20.16.0 (released July 24, 2024), so that's the correct "added" version, not v22.3.0. |
- Updated both TestContext#fullName and SuiteContext#fullName - Changed from single version to YAML list format with both v22.3.0 and v20.16.0 - Reflects that fullName was added in v22.3.0 and backported to v20.16.0
|
I have addressed both points:
|
|
Can you revert the snapshot changes? Otherwise, all good. |
Revert snapshot testing related items back to v22.3.0 as snapshot testing was never backported to v20.x. Only context.fullName was backported to v20.16.0. - Reverted ## Snapshot testing to v22.3.0 - Reverted ## snapshot to v22.3.0 - Reverted snapshot.setDefaultSnapshotSerializers to v22.3.0 - Reverted snapshot.setResolveSnapshotPath to v22.3.0 - Reverted context.assert.snapshot to v22.3.0 - Kept fullName properties with YAML list format (v22.3.0 and v20.16.0)
|
I have reverted the snapshot testing changes as requested. The snapshot-related version metadata has been restored to v22.3.0, while the |
Renegade334
left a comment
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.
Thanks!
755c51a to
6e994fe
Compare
|
CI fixed - ready for re-run |
The fullName property exists on SuiteContext at runtime but was not documented.
Tested to verify behavior:
Fixes: #60757