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 support for gulp serve --coverage coverage collection endpoint #30099

Merged
merged 3 commits into from Sep 18, 2020

Conversation

rcebulko
Copy link
Contributor

@rcebulko rcebulko commented Sep 2, 2020

When gulp serve is called with the --coverage flag, a new /coverage endpoint will be available. Coverage data can be reported to that endpoint, and it provides a simple UI to browse coverage data throughout the codebase.

Sample of interface after loading and reporting from examples/everything.amp.html:
image

Note that currently the coverage data is still reported manually; doing so automatically, either within gulp serve or for gulp e2e, will be addressed separately. This PR just adds the flag and collection endpoint.

@@ -65,6 +65,9 @@ app.use('/amp4test', require('./amp4test').app);
app.use('/analytics', require('./routes/analytics'));
app.use('/list/', require('./routes/list'));
app.use('/test', require('./routes/test'));
if (argv.coverage) {
app.use('/coverage', require('istanbul-middleware').createHandler());
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious: Does this work with the various gulp serve flags like --esm or --compiled or --sxg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--compiled yes. As for --esm and --sxg, I haven't tried those yet, but I suspect they will be slightly more complicated based on some of the related code paths I saw. I'm going to work on --esm next week to make sure that's usable and the sourcemaps function properly. As for SxG, I don't have a deep enough understanding to know whether it would have any impact, and whether SxG would be any different from a coverage perspective

Copy link
Contributor

Choose a reason for hiding this comment

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

Based on my understanding, this PR should have a similar effect on --esm and --sxg. So my guess is that if one of them works with the coverage middleware, both should. We can confirm once the initial code lands.

@rcebulko rcebulko force-pushed the middleware branch 2 times, most recently from a881d19 to 91b7a03 Compare September 15, 2020 16:43
@rcebulko
Copy link
Contributor Author

@ampproject/build-cop This failing test has been blocking this PR for almost two weeks now. Can it be skipped?
image

@zhouyx
Copy link
Contributor

zhouyx commented Sep 17, 2020 via email

@kristoferbaxter
Copy link
Contributor

Let's skip it for now to unblock new PRs.

@rcebulko can you file a PR to skip until we figure it out?

@rcebulko
Copy link
Contributor Author

rcebulko commented Sep 17, 2020

Let's skip it for now to unblock new PRs.

@rcebulko can you file a PR to skip until we figure it out?

Done #30278

@rcebulko rcebulko merged commit 73f4130 into ampproject:master Sep 18, 2020
@rcebulko rcebulko deleted the middleware branch September 18, 2020 20:10
ed-bird pushed a commit to ed-bird/amphtml that referenced this pull request Dec 10, 2020
ampproject#30099)

* add istanbul-middleware dependency

* Add coverage collection endpoint to gulp serve

* Switch middleware to devDependency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants