★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/21910Nouvelle recherche | Portail wanayoo
Skip to content

/node

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

tools: fixup docs and run known_issues by default #21910

Closed
wants to merge 1 commit into from

Conversation

Reviewers

@lance lance

@jasnell jasnell

@Trott Trott

@richardlau richardlau

@vsemozhetbyt vsemozhetbyt

Assignees
No one assigned
Labels
doc test tools
Projects
None yet
Milestone
No milestone
8 participants
@maclover7 @nodejs-github-bot @Trott @targos @lance @jasnell @richardlau @vsemozhetbyt
@maclover7
Copy link
Member

maclover7 commented Jul 20, 2018

  • Updates test/README.md with new suites
  • Fixes some outdated IGNORED_SUITES listings
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
@nodejs-github-bot

@vsemozhetbyt vsemozhetbyt added doc test tools labels Jul 20, 2018

@vsemozhetbyt

vsemozhetbyt approved these changes Jul 20, 2018
View changes

@jasnell

jasnell approved these changes Jul 20, 2018
View changes

@lance

lance approved these changes Jul 20, 2018
View changes

@maclover7 maclover7 added the author ready label Jul 20, 2018

@richardlau

richardlau approved these changes Jul 20, 2018
View changes

@Trott

Trott reviewed Jul 20, 2018
View changes

tools/test.py Outdated
@@ -1554,9 +1554,8 @@ def PrintCrashed(code):
'gc',
'internet',
'pummel',
'test-known-issues',
'known_issues',

This comment has been minimized.

Sign in to view
Copy link
@Trott

Trott Jul 20, 2018

Member

I'm surprised known_issues is in the ignore set. Any objection to removing it?

This comment has been minimized.

Sign in to view
Copy link
@maclover7

maclover7 Jul 22, 2018

Author Member

(probably) fine with removing it, but might be best to do in a separate PR for visibility.

This comment has been minimized.

Sign in to view
Copy link
@richardlau

richardlau Jul 25, 2018

Member

But if this lands then won't we stop running known_issues until the subsequent PR to remove it from this list lands?

This comment has been minimized.

Sign in to view
Copy link
@maclover7

maclover7 Jul 26, 2018

Author Member

@richardlau Ah, true -- I'll remove known_issues from the list in this PR :)
@Trott

Trott approved these changes Jul 20, 2018
View changes

Copy link
Member

Trott left a comment

LGTM, although I think we should be running known_issues, no?
@maclover7

This comment has been minimized.

Sign in to view
Copy link
Member Author

maclover7 commented Jul 25, 2018

Will land this as-is, and open up a separate PR re: test/known_issues
@maclover7

This comment has been minimized.

Sign in to view
Copy link
Member Author

maclover7 commented Jul 25, 2018

CI before landing: https://ci.nodejs.org/job/node-test-pull-request/16006/
@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

Trott commented Jul 25, 2018

Nit: fixup -> fix up in commit message to keep the verb as the first word.

@maclover7 maclover7 force-pushed the maclover7:jm-test-fixup branch from 18091e8 to 1e21c11 Jul 26, 2018

@maclover7 maclover7 changed the title doc,tools: fixup documentation tools: fixup docs and run known_issues by default Jul 26, 2018

@maclover7
tools: fix docs and run known_issues by default 
- Updates `test/README.md` with new suites
- Fixes some outdated `IGNORED_SUITES` listings
- Allows for `test/known_issues` suite to be run by default
Loading status checks…
e039bde

@maclover7 maclover7 force-pushed the maclover7:jm-test-fixup branch from 1e21c11 to e039bde Jul 26, 2018

@maclover7

This comment has been minimized.

Sign in to view
Copy link
Member Author

maclover7 commented Jul 26, 2018

Updated @richardlau @Trott, PTAL
@richardlau

richardlau approved these changes Jul 26, 2018
View changes

test/README.md
@@ -31,7 +35,7 @@ GitHub with the `autocrlf` git config flag set to true.
|sequential |Yes |Various tests that are run sequentially.|
|testpy | |Test configuration utility used by various test suites.|
|tick-processor |No |Tests for the V8 tick processor integration. The tests are for the logic in ```lib/internal/v8_prof_processor.js``` and ```lib/internal/v8_prof_polyfill.js```. The tests confirm that the profile processor packages the correct set of scripts from V8 and introduces the correct platform specific logic.|
|timers |No |Tests for [timing utilities](https://nodejs.org/api/timers.html) (```setTimeout``` and ```setInterval```).|
|v8-updates |No |Tests for V8 performance integration.|

_When a new test directory is added, make sure to update the `CI_JS_SUITES`
variable in the `Makefile` and the `js_test_suites` variable in

This comment has been minimized.

Sign in to view
Copy link
@richardlau

richardlau Jul 26, 2018

Member

This bottom note can probably be removed too as it looks like both CI_JS_SUITES and js_test_suites are default rather than a list of test directories. This can be done in another PR if you'd rather just land this PR as-is.
@maclover7

This comment has been minimized.

Sign in to view
Copy link
Member Author

maclover7 commented Jul 27, 2018

Landed in b1b2f7c, thank you for the reviews!

@maclover7 maclover7 closed this Jul 27, 2018

@maclover7 maclover7 deleted the maclover7:jm-test-fixup branch Jul 27, 2018

maclover7 added a commit that referenced this pull request Jul 27, 2018

@maclover7
tools: fix docs and run known_issues by default 
- Updates `test/README.md` with new suites
- Fixes some outdated `IGNORED_SUITES` listings
- Allows for `test/known_issues` suite to be run by default

PR-URL: #21910
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Loading status checks…
b1b2f7c

@targos targos added backport-requested-v10.x and removed author ready labels Jul 31, 2018

@targos

This comment has been minimized.

Sign in to view
Copy link
Member

targos commented Jul 31, 2018

Depends on #22039 to land on v10.x-staging

@targos targos referenced this pull request Jul 31, 2018

Closed

test: remove outdated documentation #22009

3 of 3 tasks complete

@targos targos removed the backport-requested-v10.x label Aug 2, 2018

targos added a commit that referenced this pull request Aug 2, 2018

@maclover7 @targos
tools: fix docs and run known_issues by default 
- Updates `test/README.md` with new suites
- Fixes some outdated `IGNORED_SUITES` listings
- Allows for `test/known_issues` suite to be run by default

PR-URL: #21910
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Verified
This commit was signed with a verified signature.
@targos targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
7a7c194

@rvagg rvagg referenced this pull request Aug 13, 2018

Merged

Release proposal: v10.9.0 #22295

This was referenced Aug 16, 2018

Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.