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

Testing the doctool #5955

Closed
silverwind opened this issue Mar 30, 2016 · 7 comments
Closed

Testing the doctool #5955

silverwind opened this issue Mar 30, 2016 · 7 comments
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. test Issues and PRs related to the tests.

Comments

@silverwind
Copy link
Contributor

Both the JSON and HTML output of the doctool are currently untested while tests could have prevented these recent issues:

As a start, a test could include JSON.parse and possibly some lightweight HTML validation triggered by make test-doc / vcbuild test-doc.

@silverwind silverwind added doc Issues and PRs related to the documentations. test Issues and PRs related to the tests. labels Mar 30, 2016
@silverwind silverwind added the good first issue Issues that are suitable for first-time contributors. label Mar 30, 2016
@eljefedelrodeodeljefe
Copy link
Contributor

Yeah, I am 100% +1 on this and started discussion here also nodejs/docs#93

@silverwind
Copy link
Contributor Author

I wouldn't go all out with selenium and similar browser-based tools. Just some very basic validation with lightweight tools that can live inside the repo.

@eljefedelrodeodeljefe
Copy link
Contributor

Yeah for this purpose here I agree. Maybe it's enough in general. At the time of opening the issue I didn't think that simple.
We could have the selenium stuff also, but outside of CI. Don't know how to proceed there, but starting with this here would be step in the same direction.

@firedfox
Copy link
Contributor

Also +1 on this. I just made another PR #5966 about doc. Testing the doctool would surely help a lot to prevent bugs in future.

@iankronquist
Copy link
Contributor

Hey, I'd like to take a look at this.

@iankronquist iankronquist mentioned this issue Apr 4, 2016
4 tasks
@iankronquist
Copy link
Contributor

@silverwind I just opened PR #6031 to address this issue.

iankronquist added a commit to iankronquist/io.js that referenced this issue Apr 4, 2016
Addresses nodejs#5955 on GitHub.
Test the toHTML function in html.js. Check that given valid markdown it
produces the expected html. One test case will prevent regressions of
big nodejs#5873.
iankronquist added a commit to iankronquist/io.js that referenced this issue Apr 4, 2016
Addresses nodejs#5955 on GitHub. Check that when given valid markdown it produced
valid JSON with the expected schema.
iankronquist added a commit to iankronquist/io.js that referenced this issue Apr 4, 2016
Addresses nodejs#5955. Add a build target called `make test-doc` as suggested in the
issue.
iankronquist added a commit to iankronquist/io.js that referenced this issue Apr 28, 2016
Addresses nodejs#5955 on GitHub.
* Test the toHTML function in html.js. Check that given valid markdown it
produces the expected html. One test case will prevent regressions of
big nodejs#5873.
* Check that when given valid markdown toJSON produces valid JSON with the
expected schema.
* Add doctool to the list of built in tests so it runs in CI.
Fishrock123 pushed a commit that referenced this issue May 4, 2016
* Test the toHTML function in html.js. Check that given valid markdown
  it produces the expected html. One test case will prevent regressions
  of #5873.
* Check that when given valid markdown toJSON produces valid JSON with
  the expected schema.
* Add doctool to the list of built in tests so it runs in CI.

PR-URL: #6031
Fixes: #5955
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@MylesBorins
Copy link
Contributor

Just so ya'll know this totally caught some breakages today!!!

🎉

MylesBorins pushed a commit that referenced this issue Jun 1, 2016
* Test the toHTML function in html.js. Check that given valid markdown
  it produces the expected html. One test case will prevent regressions
  of #5873.
* Check that when given valid markdown toJSON produces valid JSON with
  the expected schema.
* Add doctool to the list of built in tests so it runs in CI.

PR-URL: #6031
Fixes: #5955
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this issue Jun 23, 2016
* Test the toHTML function in html.js. Check that given valid markdown
  it produces the expected html. One test case will prevent regressions
  of #5873.
* Check that when given valid markdown toJSON produces valid JSON with
  the expected schema.
* Add doctool to the list of built in tests so it runs in CI.

PR-URL: #6031
Fixes: #5955
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this issue Jun 24, 2016
* Test the toHTML function in html.js. Check that given valid markdown
  it produces the expected html. One test case will prevent regressions
  of #5873.
* Check that when given valid markdown toJSON produces valid JSON with
  the expected schema.
* Add doctool to the list of built in tests so it runs in CI.

PR-URL: #6031
Fixes: #5955
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants