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 protocols related metrics #816

Merged
merged 23 commits into from
Nov 11, 2020
Merged

Add protocols related metrics #816

merged 23 commits into from
Nov 11, 2020

Conversation

gmetais
Copy link
Contributor

@gmetais gmetais commented Nov 2, 2020

Hi @macbre,

Congratulations for the release of 2.0.0! 🎉

While working on integrating it into YLT, I thought of this bunch of new metrics. Do you think they could be useful?

I just don't know how to add integration tests for them. Testing on a locally hosted page does not make much sense. Testing on a remote domain could change at any time... Do you have an idea?

@macbre macbre added this to the v2.1 milestone Nov 2, 2020
@macbre
Copy link
Owner

macbre commented Nov 2, 2020

@gmetais , thanks for this PR. We can try testing it using nginx and a custom SSL certificate.

@macbre
Copy link
Owner

macbre commented Nov 2, 2020

@gmetais, please run npm run make-docs to update the docs with new metrics that you've introduced.

Copy link
Owner

@macbre macbre left a comment

Choose a reason for hiding this comment

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

Let's discuss how to test these new metrics.

@gmetais
Copy link
Contributor Author

gmetais commented Nov 6, 2020

I thought I would manage to open an SSL port on nginx with a self-signed certificate, but I can't make it work. Any idea why?

@@ -6,7 +6,10 @@ services:
image: macbre/nginx-brotli:1.19.3
Copy link
Owner

@macbre macbre Nov 9, 2020

Choose a reason for hiding this comment

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

Suggested change
image: macbre/nginx-brotli:1.19.3
image: macbre/nginx-brotli:1.19.4

This solves problems with SSL (1.19.3 image had an issue with nginx build) - see macbre/docker-nginx-http3#15

Copy link
Owner

@macbre macbre Nov 9, 2020

Choose a reason for hiding this comment

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

@gmetais - now we have

 localhost but https version
    ✗ should be generated 
        » No error should be thrown: got Error: net::ERR_CERT_AUTHORITY_INVALID at https://127.0.0.1:8889 // /home/runner/work/phantomas/phantomas/node_modules/vows/lib/assert/macros.js:31

We need to pass a different option to puppeteer -> https://github.com/puppeteer/puppeteer/blob/main/docs/api.md#puppeteerlaunchoptions (ignoreHTTPSErrors)

Copy link
Owner

Choose a reason for hiding this comment

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

FIxed in #821

test/integration-spec.yaml Outdated Show resolved Hide resolved
test/integration-spec.yaml Outdated Show resolved Hide resolved
test/integration-spec.yaml Outdated Show resolved Hide resolved
@macbre
Copy link
Owner

macbre commented Nov 9, 2020

Tests now pass 🎉

@macbre macbre merged commit df62e01 into macbre:devel Nov 11, 2020
@gmetais gmetais deleted the protocols branch November 25, 2020 21:05
@gmetais
Copy link
Contributor Author

gmetais commented Nov 25, 2020

Huge! Thanks for the fixes!

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.

2 participants