Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add JUnit output and expand JSON output #275

Merged
merged 4 commits into from
Jan 29, 2023

Conversation

shivjm
Copy link
Contributor

@shivjm shivjm commented Jan 21, 2023

This PR does three things:

  1. Allow including successful results in JSON output. (Fixes Add support for outputing to JSON all crawled URLs (including 200 ones) #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
Copy link
Owner

raviqqe commented Jan 29, 2023

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.

@raviqqe raviqqe mentioned this pull request Jan 29, 2023
@raviqqe raviqqe merged commit 822b6b0 into raviqqe:main Jan 29, 2023
raviqqe added a commit that referenced this pull request Jan 29, 2023
* Allow including successful results in JSON output

* Flatten all results into single array in JSON output

* Add JUnit output flag

* Track elapsed time when checking links and include in JUnit output

* Disable spell check

* Disable more spell check

* Remove unused type

---------

Co-authored-by: Shiv Jha-Mathur <[email protected]>
@shivjm shivjm deleted the add-junit-output-and-expand-json branch January 29, 2023 09:22
@shivjm
Copy link
Contributor Author

shivjm commented Jan 29, 2023

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
Copy link
Owner

raviqqe commented Jan 29, 2023

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!

@raviqqe raviqqe mentioned this pull request Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for outputing to JSON all crawled URLs (including 200 ones)
2 participants