Implement issue #276 exclude certain URLs from checking, also impleme… #368
Conversation
…plement hosts to exclude
|
Thanks for your contribution, @elebeida. Could you please check the test coverage (cf. SonarQube report). I see some tests but mostly integration tests, but the exclusion logic should be covered by unit tests if possible. Additionally, it would be nice if you could also add some documentation wrt the new parameters (README of Gradle Plugin). |
self-check/build.gradle
Outdated
| @@ -11,6 +11,9 @@ htmlSanityCheck { | |||
|
|
|||
| failOnErrors = true | |||
|
|
|||
| urlsToExclude = [ "https://www.aim42.org/"] | |||
| hostsToExclude = [ "www.aim42.org" ] | |||
|
|
|||
| logger.quiet "HSC version: ${htmlSanityCheckVersion}" | |||
There was a problem hiding this comment.
The purpose of the self check is to check the HSC documentation itself.
And it should not exclude essential links in the documentation.
Please drop this.
| <a href="http://included.com/page">Included Host</a> | ||
| </body> | ||
| </html> | ||
| """ |
There was a problem hiding this comment.
As far as I can see these HTMLs are only used in HtmlSanityCheckTaskFunctionalSpec (despite the other final statics that are used in both derived classes. If the code is only used in one derived class, move it there please.
|
Please rethink the approach, @elebeida towards a regular expression based implementation. Now that I look through your implementation and the original requirement of @rdmueller (who comes with a strong docToolchain back ground), I think that a regular expression is more general as it might cover URLs as well as Hosts. |
…ment unit test and write documentation
|
i have now implemented support for regular expression, as you asked for. |
| @@ -189,6 +189,28 @@ include::../htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/tools/W | |||
| The lists shown above are the default HTTP response codes handled by HSC. | |||
| The mentioned configurations effectively move the configured codes around, i.e., if you add `308` to `httpErrorCodes` it is automatically removed from its default list (`httpWarningCodes`). | |||
| **** | |||
| `urlsToExclude` (optional):: A list URLs that should be excluded from the sanity check. These URLs can be written as regular expressions. | |||
There was a problem hiding this comment.
Thanks for the update, but did you see my general comment about URLs to exclude vs. Hosts to exclude, @elebeida?
I think we could cover both by just one regex based property, e.g., exclude.
By using regular expressions, It could contain full (or even partly) URLs (with schema, port etc.) as well as simple host names.
For example,
exclude=^.*internal\.example-to-exclude\.com.+would exclude everything from this host, whileexclude=^https?://internal\.example-to-exclude\.com.*would exclude only http and https based URLs, while allowing other schema likeftp(if that makes sense at all).
There was a problem hiding this comment.
yes, but i might have misunderstood it. I will make another try. Thanks for the explanation :)
|
Implement issue #276 exclude certain URLs from checking, also implement hosts to exclude