search

snitch-org / snitch

Lightweight C++20 testing framework.
Boost Software License 1.0
266 stars 10 forks source link

Draft: Feat/catch2 compat #169

Closed CrustyAuklet closed 1 month ago

CrustyAuklet commented 6 months ago

Very rought draft, but I wanted to open it so others can look and comment.

This is work to make the Catch2 XML output match up with Catch2. Like the other open MRs I notices this when integrating with my teams IDE and (more importantly) CI test reporting systems.

Without these changes CI and IDEs work ok, but failures in a section just fail the whole test. When we were using Catch2 a failure would be specific to a section allowing us to quickly drill down to the problem. So looking into the difference it is just the lack of section output in the "high" verbosity mode. Catch2 outputs section XML data by default.

It was fairly easy to make it work with basic testing, pass and fail. There are more complex edge cases around skips, nested sections, exceptions being thrown, and different verbosity levels. I am still ironing those out and need to make sure there is testing for it all.

As I have time this week i will add some examples here, and some pictures to show the different in CI/IDEs.

cschreib commented 6 months ago

Thanks for spotting this. I could have sworn I got the output for sections to match Catch2 when I last tested this; but perhaps I had looked at a too simple case and missed the bigger picture.

As I have time this week i will add some examples here, and some pictures to show the different in CI/IDEs.

It would be very interesting to compare the actual XML output between the two in a real-world case; the test project I use for testing and benchmarks doesn't actually use sections.

CrustyAuklet commented 6 months ago

thanks for the feedback, I will look it over in depth this coming week.

I've been working on making tests and comparing Catch2 to Snitch output in a repo on my account here: https://github.com/CrustyAuklet/snitch-xml-output. So far it's pretty close but I will have to add some more variations, especially to cover the exception stuff.

cschreib commented 5 months ago

Hi there! Just wondering if this was still on your mind. Let me know if not, and I can take over.

CrustyAuklet commented 5 months ago

It is, I have just had a ton on my plate. I am going to try to re-familiarize myself with it this afternoon and see if have any questions on your feedback.

I won't be offended at all if you think it's an easy fix for you to do and you want to get it done 😄

CrustyAuklet commented 4 months ago

fixed some of the small easy stuff and rebased, but spent a lot of time running some example tests in that other repo with exceptions. I think most of this code needs to be moved to the registry like you said.

cschreib commented 2 months ago

Thanks for looking at it further. Does snitch reporting work as expected in your tests now? If so, I can do another pass of review, and if all looks fine, I can also take care of the final refactoring.

FYI: I just hit some of the issues you were having, so I can confirm (if this was even needed!) that things are currently broken on the main branch. I'm implementing a test runner for SublimeText, and for the sake of the exercise, I implemented a test adaptor for Catch2 first, and then tried to use it with snitch without touching it. Didn't work out of the box! I'm hoping this will be solved with your PR.

CrustyAuklet commented 2 months ago

Without exceptions it works as expected and I am using it on some of my work projects. But with exceptions It doesn't match up with the output of Catch2, and that is where I got a bit stuck and maybe the code does need to move into the registry as you suggested.

The test using exceptions in the repo https://github.com/CrustyAuklet/snitch-xml-output shows some of the differences. Catch2 is more verbose but the real issue is that is seems that exceptions thrown from within sections are not output until the parent section.

Here are the output XML files from that test: exceptions.catch2.txt exceptions.snitch.txt

cschreib commented 2 months ago

I believe the remaining issues are now sorted if you try the branch cschreib/catch2-compat. I created a PR for it https://github.com/snitch-org/snitch/pull/183 to get the CI runs. If you have no objection, I suggest switching to the new PR and closing this one (unless you feel like doing the git magic to pull my changes back into your branch; you'll need to rebase though). If you wanted to review the changes I pushed over your last commit, feel free!

By the way, if you need to do another PR later for some other change, you should have permissions to create a branch direclty in this repo without having to work from a fork. This would make it possible for us to work on the same branch when needed (which I could not do here, since this PR's branch belongs exclusively to you).

cschreib commented 1 month ago

Superseded by https://github.com/snitch-org/snitch/pull/183; thanks again for kick-starting this!