Skip to content
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

[node:test] Incorrect SuiteContext description #45641

Closed
Semigradsky opened this issue Nov 27, 2022 · 5 comments · Fixed by #45687
Closed

[node:test] Incorrect SuiteContext description #45641

Semigradsky opened this issue Nov 27, 2022 · 5 comments · Fixed by #45687
Labels
good first issue Issues that are suitable for first-time contributors. test_runner Issues and PRs related to the test runner subsystem.

Comments

@Semigradsky
Copy link
Contributor

Affected URL(s)

https://nodejs.org/docs/latest-v18.x/api/test.html#describename-options-fn

Description of the problem

Docs said that tha callback function in describe:

declaring all subtests and subsuites. The first argument to this function is a SuiteContext object.

But in fact SuiteContext is not the first argument. SuiteContext is this context:

import { describe } from 'node:test'

describe(function(foo) {
    console.log('First argument: ', foo)
    console.log('this: ', this)
})

> First argument:  []
> this:  { signal: AbortSignal { aborted: false }, name: '<anonymous>' }
@Semigradsky Semigradsky added the doc Issues and PRs related to the documentations. label Nov 27, 2022
@Semigradsky
Copy link
Contributor Author

But TestContext is the first argument:

import { test } from 'node:test'

test(function(foo) {
    console.log('First argument: ', foo)
    console.log('this: ', this)
})

> First argument:  TestContext {}
> this:  TestContext {}

@cjihrig
Copy link
Contributor

cjihrig commented Nov 27, 2022

@MoLow it looks like the SuiteContext language was added in 389b7e1. It doesn't look like SuiteContext exists in the code though. Should all references to SuiteContext be replaced with TestContext?

@cjihrig cjihrig added the test_runner Issues and PRs related to the test runner subsystem. label Nov 27, 2022
@MoLow
Copy link
Member

MoLow commented Nov 28, 2022

@cjihrig it is simply generated as an object literal

getRunArgs() {
return { ctx: { signal: this.signal, name: this.name }, args: [] };
}

@cjihrig
Copy link
Contributor

cjihrig commented Nov 29, 2022

I think we should either change the docs to remove the use of SuiteContext since it doesn't really exist as a class or update the code to make it an actual class. I think updating the code would be easiest in this case.

@MoLow MoLow added diag-agenda Issues and PRs to discuss during the meetings of the diagnostics working group. good first issue Issues that are suitable for first-time contributors. and removed doc Issues and PRs related to the documentations. diag-agenda Issues and PRs to discuss during the meetings of the diagnostics working group. labels Nov 29, 2022
debadree25 added a commit to debadree25/node that referenced this issue Nov 30, 2022
added SuiteContext class to replace object literal

Fixes: nodejs#45641
Refs: nodejs#45641 (comment)
@debadree25
Copy link
Member

Have added PR for the same @cjihrig

nodejs-github-bot pushed a commit that referenced this issue Dec 2, 2022
added SuiteContext class to replace object literal

Fixes: #45641
Refs: #45641 (comment)
PR-URL: #45687
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
targos pushed a commit that referenced this issue Dec 12, 2022
added SuiteContext class to replace object literal

Fixes: #45641
Refs: #45641 (comment)
PR-URL: #45687
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
added SuiteContext class to replace object literal

Fixes: #45641
Refs: #45641 (comment)
PR-URL: #45687
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
added SuiteContext class to replace object literal

Fixes: #45641
Refs: #45641 (comment)
PR-URL: #45687
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
danielleadams pushed a commit that referenced this issue Jan 3, 2023
added SuiteContext class to replace object literal

Fixes: #45641
Refs: #45641 (comment)
PR-URL: #45687
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
danielleadams pushed a commit that referenced this issue Jan 4, 2023
added SuiteContext class to replace object literal

Fixes: #45641
Refs: #45641 (comment)
PR-URL: #45687
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
danielleadams pushed a commit that referenced this issue Jan 5, 2023
added SuiteContext class to replace object literal

Fixes: #45641
Refs: #45641 (comment)
PR-URL: #45687
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that are suitable for first-time contributors. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants