wjdp / htmltest

:white_check_mark: Test generated HTML for problems

MIT License

325 stars 54 forks source link

Adding IgnoreSSLVerify option #113

Closed jgazeau closed 5 years ago

jgazeau commented 5 years ago

Patch to add IgnoreSSLVerify option (issue #112 )

wjdp commented 5 years ago

PR makes sense, simple and effective! Unsure why the tests are failing, having a quick look now.

I've tweaked the README wording and would be nice to have a test case for this option.

wjdp commented 5 years ago

Is failing on master as well, some package or the Travis environment must have changed. Very odd.

jgazeau commented 5 years ago

It seems that they changed something in the golan url_test.go and add a new check for special char (check : https://github.com/golang/go/commit/829c5df58694b3345cb5ea41206783c8ccf5c3ca#diff-6f43cc724ae52e4acba8855ce391e144) I agree that it would be nice to have a test to check the IgnoreSSLVerify option. I'll check this and see how to do it.

wjdp commented 5 years ago

@jgazeau Can you rebase this on master?

codecov[bot] commented 5 years ago

Codecov Report

Merging #113 into master will increase coverage by 0.02%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #113      +/-   ##
==========================================
+ Coverage   86.53%   86.56%   +0.02%     
==========================================
  Files          20       20              
  Lines        1062     1064       +2     
==========================================
+ Hits          919      921       +2     
  Misses        136      136              
  Partials        7        7

Impacted Files	Coverage Δ
htmltest/options.go	`90.14% <100%> (+0.14%)`	:arrow_up:
htmltest/htmltest.go	`97.84% <100%> (+0.01%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4b5831f...c580c11. Read the comment docs.

jgazeau commented 5 years ago

@wjdp Rebase done. Haven't checked to implement a test for this option yet.

wjdp commented 5 years ago

Thanks, I'll hold off merging until you've done a test case.

jgazeau commented 5 years ago

@wjdp,

I add two test cases (TestSelfSignedLink and TestSelfSignedLinkIgnoreSSLVerify) in the check-link_test.go.

TestSelfSignedLink is checking if a link with a selfsigned certificate is returning an error with the IgnoreSSLVerify=false
TestSelfSignedLinkIgnoreSSLVerify is checking if a link with a selfsigned certificate is not returning any error with the IgnoreSSLVerify=true

For the test I use the self-signed link of https://badssl.com/.

FYI: To be sure, I run one pipeline to check the job is throwing errors by swaping the IgnoreSSLVerify boolean value : https://travis-ci.org/jgazeau/htmltest/jobs/524849941 (Line 764)

Is that Ok for you ?

wjdp commented 5 years ago

@jgazeau Whoops, sorry I missed your last comment. Thanks for the PR!