wjdp / htmltest

:white_check_mark: Test generated HTML for problems
MIT License
323 stars 54 forks source link

Ignore internal urls #169

Closed divinerites closed 3 years ago

divinerites commented 3 years ago

Hello,

Following #168 I made a small patch. It seems that for this use case, we cannot use the IgnoreURLs because it uses regexp and this is I think too violent for internal urls.

So I added the internal equivalent IgnoreInternalURLs and just done an equal test for url. Seems to work fine for what I tested.

I also added some tests, but as said I'm a golang noob, so I don't know if those tests suits your quality needs. may be you have to add/reinforce those tests.

Hope this is helpful. Done with go version go1.16 darwin/amd64

Solve #168

divinerites commented 3 years ago

Hello,

Any news for this PR ? Is there something more that you need ?

wjdp commented 3 years ago

Hey @divinerites sorry for the delay on this. I'm away the rest of this week, will try and get to this next week.

divinerites commented 3 years ago

@wjdp thanks for your comments.

Regarding your request for "test of this running on an HTML file", I'll look if i can, but I'm afraid this is not in my go skills. Will try to look at it tho.

wjdp commented 3 years ago

@divinerites Have a look at the existing tests, it should be straightforward:

There's many examples of this, like:

func TestAnchorDirectoryNoTrailingSlash(t *testing.T) {
    // fails for internal linking to a directory without trailing slash
    hT := tTestFile("fixtures/links/link_directory_without_slash.html")
    tExpectIssueCount(t, hT, 1)
    tExpectIssue(t, hT, "target is a directory, href lacks trailing slash", 1)
}

func TestAnchorDirectoryNoTrailingSlashOption(t *testing.T) {
    // passes for internal linking to a directory without trailing slash when asked
    hT := tTestFileOpts("fixtures/links/link_directory_without_slash.html",
        map[string]interface{}{"IgnoreDirectoryMissingTrailingSlash": true})
    tExpectIssueCount(t, hT, 0)
}
divinerites commented 3 years ago

@wjdp I've done it. .build.sh seems to work well. But never properly used tests before. So I guess you have to check if my tests are good and efficient.

divinerites commented 3 years ago

OK. I understood better how to run test. Corrected some wrong syntax.

Tests seems OK except one :

=== RUN   TestIsInternalURLIgnored
    assert.go:73: options_test.go:71: url ignored unexpectedly got: false.
--- FAIL: TestIsInternalURLIgnored (0.00s)
divinerites commented 3 years ago

OK. All test PASS now.

Sorry for being so slow, but this is quite new to me. Anyway, tests should be good now.

mtlynch commented 3 years ago

Cool, thanks @divinerites and @wjdp!