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

Export coverage #194

Merged
merged 5 commits into from
Jul 1, 2018
Merged

Export coverage #194

merged 5 commits into from
Jul 1, 2018

Conversation

vweevers
Copy link
Member

@vweevers vweevers commented Jul 1, 2018

Closes #162.

This disables coverage by default. If --coverage is passed, airtap collects code coverage per browser, written to .nyc-output/airtap-<hash>.json in Istanbul 1.0 format. One can generate reports with nyc report (nyc takes care of merging files from multiple browsers). In local mode, airtap writes to .nyc-output/airtap-local.json (so that no cleanup is necessary between repeated runs, e.g. if you refresh the page).

Usage in CI is airtap --sauce-connect --loopback airtap.local --coverage test.js, and if you want, add a Travis hook to post results to coveralls:

after_success: npm run coverage
"scripts": {
  "coverage": "nyc report --reporter=text-lcov | coveralls"
}

Later we should:

  • Remove the "Code coverage" tab from the UI (IMO)
  • Remove previous files before all tests start (this only matters when running locally and you also run node tests and already have files in .nyc-output)
  • Make it more generic (not tied to nyc)
  • Move the middleware to a module
  • Support coverage in electron

@vweevers vweevers added this to Backlog in Airtap via automation Jul 1, 2018
@vweevers vweevers moved this from Backlog to Review in Airtap Jul 1, 2018
Copy link
Member

@ralphtheninja ralphtheninja left a comment

Choose a reason for hiding this comment

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

Super!

@ralphtheninja
Copy link
Member

Later we should:

Remove the "Code coverage" tab from the UI (IMO)
Remove previous files before all tests start (this only matters when running locally and you also run node tests and already have files in .nyc-output)
Make it more generic (not tied to nyc)
Move the middleware to a module
Support coverage in electron

@vweevers Please create some issues for this.

@vweevers
Copy link
Member Author

vweevers commented Jul 1, 2018

Please create some issues for this.

Of course. I figured I'd wait for feedback first.

@vweevers
Copy link
Member Author

vweevers commented Jul 1, 2018

I think before merging, I want to test this against memdown (and Sauce Labs).

@vweevers vweevers merged commit 65be371 into master Jul 1, 2018
Airtap automation moved this from Review to Done Jul 1, 2018
@vweevers vweevers deleted the coverage branch July 1, 2018 11:32
@ralphtheninja
Copy link
Member

@vweevers Maybe we should take parts of this PR description and put into the README. WDYT?

@vweevers
Copy link
Member Author

vweevers commented Jul 2, 2018

Sure, why not :)

@ralphtheninja ralphtheninja mentioned this pull request Jul 2, 2018
@ralphtheninja
Copy link
Member

Hmm, unsure where to put the information. The doc/ folder is kind of hidden. Maybe we should link from the README to those .md files.

@ralphtheninja
Copy link
Member

Aah nevermind, some are linked already.

@vweevers
Copy link
Member Author

vweevers commented Jul 2, 2018

See also #33

@ralphtheninja
Copy link
Member

See also #33

Adding it to the milestone.

@vweevers
Copy link
Member Author

vweevers commented Jul 6, 2018

Opened issues:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Airtap
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants