search

fffunction / backstop-crawl

🕷 Crawls a site to generate a backstopjs config file with scenarios pre-populated
38 stars 18 forks source link

Fix missing reference url slash #26

Closed ataylorme closed 6 years ago

ataylorme commented 6 years ago

Addresses #25 by replacing one or more trailing slashes in referenceUrl with a single trailing slash.

danreeves commented 6 years ago

This doesn't solve the issue I had where the URL has a trailing slash and so does the referenceUrl, resulting in two slashes at the end of the domain. Right?

I think it was a mistake on my part to hack around this with a regex. I'd rather fix the url we're placing in the currentScenario with normalize-url. e.g.http://requirebin.com/?gist=b34c314281156c23c0e21dc2dd970c4b

ataylorme commented 6 years ago

@danreeves makes sense to me. I think we will have to be careful as removeTrailingSlash says Note: Trailing slash is always removed if the URL doesn't have a pathname.

This means http://test.com/////// will turn into http://test.com, so we will need to add a trailing slash back.

danreeves commented 6 years ago

If we do at the point where we have the full url, before https://github.com/fffunction/backstop-crawl/blob/master/lib/crawl.js#L116 it should be fine.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 6d15dba68ba107da31018b24dc03d80c4f32d7c2 on ataylorme:missing-reference-url-slash into e6ecdd8bd2f9128821ff052e479b5059262d32fe on fffunction:master.

ataylorme commented 6 years ago

@danreeves should be all set now. Thanks for talking through this with me!