search

raviqqe / muffet

Fast website link checker in Go
MIT License
2.49k stars 96 forks source link

Add JUnit output and expand JSON output #275

Closed shivjm closed 1 year ago

shivjm commented 1 year ago

This PR does three things:

  1. Allow including successful results in JSON output. (Fixes #214.)
  2. Add JUnit output with a --junit flag. (Allows GitLab to parse output. Does not include skipped links.)
  3. Track elapsed time for each link and each page. (Used in JUnit output.)

I’m not a Go developer, so let me know if there are any best practices or style guidelines I’ve ignored (most probably when updating existing tests to add durations with lots of repetition) and I’ll try to fix them. I modeled the JUnit addition after the existing JSON output and divided the changes into atomic commits as far as possible. There are no errors or warnings, the tests pass, and the JUnit output seems to be correct (I used broken-links-inspector’s JUnit output as a point of reference).

raviqqe commented 1 year ago

Hi! Thank you for your contribution and sorry for my delayed review. The changes look great overall! Just so you know, given your changes, I think I need to refactor some options of Muffet to keep it consistent. For example, I will probably rename the --include-success-in-json to --experimental-verbose-json for now. Then, I will integrate it into the normal --verbose option in Muffet v3.

I just have one question. Do you think there are any use cases for elapsed times in JUnit outputs? I'm not sure if anyone is interested in using Muffet as a performance tester because latencies in UX are quite different from network speeds. They are also variable against infrastructure and its state. For example, cache misses or hits in edge computation changes latencies a lot.

shivjm commented 1 year ago

Thank you for the review and merge. 😊 I’m relieved to hear the code wasn’t unusable.

For example, I will probably rename the --include-success-in-json to --experimental-verbose-json for now. Then, I will integrate it into the normal --verbose option in Muffet v3.

Sure, makes sense. I tried to pick the most obvious names I could think of, and I’m more than happy to defer to your expertise when it comes to the long-term interface.

I just have one question. Do you think there are any use cases for elapsed times in JUnit outputs?

I haven’t used them myself so far because I couldn’t access them in other checkers, and I honestly don’t know whether anyone else uses them. It just seemed like a neat thing to include for anyone who wants it, since the format supports it. If it seems superfluous to you, or if you feel it adds undue complexity, please don’t hesitate to remove it!

raviqqe commented 1 year ago

I've actually decided to remove the code for performance time tracking (#278) for now because Muffet depends on Go's runtime for scheduling everything in the current implementation and users can't really know if they are tracking times taken by programs (e.g. performance of Muffet) or network latencies (e.g. performance of their websites.) Basically, users need to know whether the capacity of a machine where Muffet is running on is great enough compared with a size of their website ahead. But thanks for your feedback anyway!