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

Add tests to detect missing API documentation #16912

Closed
wants to merge 5 commits into from
Closed

Add tests to detect missing API documentation #16912

wants to merge 5 commits into from

Conversation

jenweber
Copy link
Contributor

@jenweber jenweber commented Aug 25, 2018

What it does

This PR includes a test of the highest-level yuidoc json. While a passing test does not guarantee that the docs will be rendered in the api app, it should help catch things that would definitely be missing.

Why it's needed

Historically, ember.js minor releases have caused a regression in the API docs for our users of https://emberjs.com/api/ and a scramble to make a patch release.

Changes made

  • added a test to bin/yuidoc-tests.js
  • added a command to package.json scripts,test:docs which builds the docs before running tests with qunit: ember ember-cli-yuidoc qunit bin/yuidoc-tests.js

Help plz

  • Is bin the right place for this test to go?
  • What needs to happen to make the test:docs command run?

@kategengler
Copy link
Member

I'm pretty sure bin is the right place for the test, but also not sure the correct place. I would have said node-tests but those seem to exclusively blueprint tests.

To have the test:docs command run, I think you want to add a test suite case around here: https://github.com/emberjs/ember.js/blob/master/bin/run-tests.js#L213 And add a stage with that test suite here: https://github.com/emberjs/ember.js/blob/master/.travis.yml#L75

It should probably be an early stage ala code quality and basic tests, so that CI runs fail fast if it is going to fail.

@kategengler kategengler requested a review from rwjblue August 25, 2018 20:04
@jenweber
Copy link
Contributor Author

Thanks Katie! I'll try adding it into those files. I think maybe it should go before browserstack, but after the basic/code quality tests, since building the yuidocs takes a little while.

@kategengler
Copy link
Member

Sorry, looking closer, there are two stages: basic test and additional tests, and each env within them is parallelized (see https://travis-ci.org/emberjs/ember.js). How long does it take to build the yuidoc? If it is shorter than ~6 minutes (the time for the each-package-tests part of basic test stage), I think it should be in the basic test stage.

@ef4
Copy link
Contributor

ef4 commented Aug 26, 2018

We may want to combine idea from this PR and #16910.

This one is more thorough in that it looks at many categories, not just classitems. But the other one gives more specific feedback about which items disappeared or were added.

@jenweber
Copy link
Contributor Author

Lol, Ed, thank you for writing that! This is what I get for not looking at the PR queue. I'm in favor of closing my PR and adding the other checks into your test.

@jenweber
Copy link
Contributor Author

Closed in favor of #16910

@jenweber jenweber closed this Aug 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants