FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: add `How to write a Node.js test` guide by santigimeno · Pull Request #6984 · nodejs/node · GitHub

/ node Public

doc: add How to write a Node.js test guide - #6984

Merged
santigimeno merged 1 commit into
nodejs:masterfrom
santigimeno:test_guid
May 30, 2016
ConversationCommits (1)ChecksFiles changed
Merged

doc: add How to write a Node.js test guide#6984
santigimeno merged 1 commit into
nodejs:masterfrom
santigimeno:test_guid

Conversation

santigimeno commented May 26, 2016

Copy link
Copy Markdown
Member
Checklist
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

doc, test

Description of change

Refs: nodejs/testing#30

/cc @nodejs/testing @nodejs/documentation

nodejs-github-bot added the doc Issues and PRs related to the documentations. label May 26, 2016
santigimeno mentioned this pull request May 26, 2016
Closed
santigimeno added the test Issues and PRs related to the tests. label May 26, 2016
addaleax reviewed May 26, 2016
View reviewed changes
Comment thread doc/guides/writing_tests.md Outdated

addaleax May 26, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

nit: I’d prefer s/They/Tests/ because there’s no plural tests before this in the text.

addaleax commented May 26, 2016

Copy link
Copy Markdown
Member

There’s a “test” missing in the commit message/PR title.

Generally looking good, nice work!

addaleax commented May 26, 2016

Copy link
Copy Markdown
Member

Oh, and I think linking this from the corresponding CONTRIBUTING.md section would be really helpful.

Fishrock123 changed the title doc: add How to write a Node.js guide doc: add How to write a Node.js test guide May 26, 2016
yorkie reviewed May 26, 2016
View reviewed changes
Comment thread doc/guides/writing_tests.md Outdated

yorkie May 26, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Would "How to write a test for the Node.js project" be better because I thought this is for how to write a test case in Node.js community, not the Node.js project in my first sight on this title. :-)

santigimeno force-pushed the test_guid branch 3 times, most recently from f5bd5fe to 8259de3 Compare May 27, 2016 16:29

santigimeno commented May 27, 2016 •
edited
Loading

Copy link
Copy Markdown
Member Author

PR updated with @addaleax and @yorkie suggestions. I'll rebase the PR before merging. Thanks!

addaleax commented May 27, 2016

Copy link
Copy Markdown
Member

LGTM and I still think linking this from the CONTRIBUTING.md is a good idea :)

santigimeno commented May 27, 2016

Copy link
Copy Markdown
Member Author

Oh! I forgot. Adding it...

santigimeno force-pushed the test_guid branch 6 times, most recently from 6b5c7a0 to 9fec0a5 Compare May 27, 2016 17:29

santigimeno commented May 27, 2016

Copy link
Copy Markdown
Member Author

Link added to CONTRIBUTING.md. @addaleax PTAL.

addaleax reviewed May 27, 2016
View reviewed changes
Comment thread CONTRIBUTING.md Outdated

addaleax May 27, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

There are only very few cases of explicit license notifications in the test code base (and in general) left, so I’d drop the mention here.

santigimeno May 27, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Maybe dropping the whole parenthesis? The common includes info is already in the guide.

addaleax May 27, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@santigimeno Oh, sorry, didn’t see this was taken directly from the existing text. But yeah, +1 to dropping the whole parenthesis here.

addaleax commented May 27, 2016

Copy link
Copy Markdown
Member

LGTM with a comment

santigimeno commented May 27, 2016

Copy link
Copy Markdown
Member Author

Updated. Thanks!

yorkie reviewed May 27, 2016
View reviewed changes
Comment thread doc/guides/writing_tests.md Outdated

yorkie May 27, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

missing javascript?

Trott commented May 27, 2016

Copy link
Copy Markdown
Member

LGTM.

There are definitely some more improvements that can happen in the CONTRIBUTING doc but it's probably best for that to happen in another PR.

santigimeno commented May 27, 2016

Copy link
Copy Markdown
Member Author

Added the missing javascript. Thanks

santigimeno commented May 27, 2016

Copy link
Copy Markdown
Member Author

There are definitely some more improvements that can happen in the CONTRIBUTING doc but it's probably best for that to happen in another PR.

The sentence about the location of the tests could be changed and then explained in some depth in the guide.

but it's probably best for that to happen in another PR.

Agreed

doc,test: add How to write a Node.js test guide
b83b363 
PR-URL: nodejs#6984
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>

santigimeno commented May 30, 2016

Copy link
Copy Markdown
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/2862/. All green except unrelated failures in some ARM bots. Landing

santigimeno merged commit b83b363 into nodejs:master May 30, 2016
santigimeno deleted the test_guid branch May 30, 2016 08:27

santigimeno commented May 30, 2016

Copy link
Copy Markdown
Member Author

Landed in b83b363. Thanks!

Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 30, 2016
doc,test: add How to write a Node.js test guide
4d5ea06 
PR-URL: nodejs#6984
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
rvagg pushed a commit that referenced this pull request Jun 2, 2016
doc,test: add How to write a Node.js test guide
b2c7d46 
PR-URL: #6984
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
MylesBorins mentioned this pull request Jul 11, 2016
Closed
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
doc,test: add How to write a Node.js test guide
8f0b535 
PR-URL: #6984
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
doc,test: add How to write a Node.js test guide
2dd0a2d 
PR-URL: #6984
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
doc,test: add How to write a Node.js test guide
591876e 
PR-URL: #6984
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
MylesBorins mentioned this pull request Jul 12, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
doc,test: add How to write a Node.js test guide
521f74a 
PR-URL: #6984
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
doc,test: add How to write a Node.js test guide
15bb0be 
PR-URL: #6984
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL