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

Support for multiple reporters was removed #1262

Closed
rcdailey opened this issue Apr 24, 2018 · 4 comments
Closed

Support for multiple reporters was removed #1262

rcdailey opened this issue Apr 24, 2018 · 4 comments
Labels

Comments

@rcdailey
Copy link

Support for multiple reporters was removed in commit 414dcae by @horenmar. I am no longer able to do this:

   Catch::Session catch_session;

   result = catch_session.applyCommandLine(argc, argv);
   if (result != 0)
   {
      return result;
   }

   catch_session.configData().reporterNames = {
      "console",
      "ErrorLogReporter",
   };

In my case, I want to supplement the existing console reporter with another one. In other words, I want output from both reporters to be present. I do not see a way to retain this functionality since reporterNames was changed to a single string.

@rcdailey
Copy link
Author

rcdailey commented Apr 24, 2018

Is the intent that I should use a listener instead? I have to say it's a giant pain in the ass that reporters aren't properly documented, and there's no tutorial / reference for listeners vs reporters. Reporters API has been very volatile and it's been difficult to keep track of the general direction the Catch developers are taking things.

For example the commit I referenced has no helpful description. There's no issue or pull request I could find that explained the purpose of the change and what users should do that depended on the old interface.

@horenmar
Copy link
Member

The problem with multiple reporters was that they never worked properly. That is, if they had differing internal requirements (e.g. whether they should always be notified of an assertion, whether they desire stdout and stderr to be captured, etc), the configuration of only one of them was used.

We could've jury-rigged checking to see if the actual reporters have different configurations and error out only if their configurations are not compatible, but that would still leave open the question of what to do with the -o flag (pretty much no 2 reporters provide sane output when they are interleaved to the same output). In the end we've decided to just disallow multiple reporters and explicitly force Listeners instead.

Reporters API has been very volatile

This is interesting, the only change to that I remember is adding a (default-implemented) new method that is called in the event of fatal error. What changes have you ran into?

@horenmar horenmar added the Query label Apr 26, 2018
@rcdailey
Copy link
Author

rcdailey commented Apr 26, 2018

Hey thanks for responding!

Your explanation helps. I actually moved my reporter to a listener and it worked out fine. I think the main issue here is documentation: It's not clear what I should do as a user depending on my goals. The cases you mentioned as motivation for reducing reporters down to a single one did not apply to me. I simply needed to supplement test diagnostic output. For these simple cases, I think a reporter makes more sense (just based on the name of the class) since I'm reporting something, not really doing logic-based stuff (to me this is what listeners appear to be intended for). Not sure what the purpose of the two is though.

To my complaints about API: I don't remember specifics since it was a year or so ago. But I remember most of it being maybe the upgrade to 2.0 catch. At one point, I remember virtual overrides being different. Base class function signatures changing. I also remember the way you include catch.hpp being a little complicated when you have multiple translation units.

I know it sounded like I was venting, but my intention was to be a bit more constructive: I'm not bothered about the change itself. What bothers me the most is lack of documentation. There are questions that are very difficult to find answers to right now:

  • When do I choose a reporter versus a listener? What are the guidelines? Examples of tasks for each?
  • Which reporter base class do I derive from and how does the context of what I'm trying to accomplish affect that decision?
  • What are the semantics behind each of the callbacks (e.g. testCaseStarted)? For example, I had to find out the hard way that when you have a SECTION within a TEST_CASE, and you try to delete global objects that local objects of the parent TEST_CASE depended on in sectionEnded(), this caused undefined behavior since the destructors of local objects in that parent TEST_CASE have not been invoked yet. This is part of the reason for my init/deinit pull request.
  • Where are the examples of good uses for reporters, how to leverage them in a useful way, such as appending output to the logs?
  • When an interface change happens in listener or reporter base classes, what actions should the user take? Why are changes to those interfaces not backward-compatible?

I ultimately had to dig into the catch code base to find answers to most of these. But definitely having documentation in the form of a tutorial for reporters & listeners with lots of examples would be amazing.

As it relates to this issue, I have one other concern. While switching from reporter to a listener resolved the requirement I had for multiple reporters, I am now no longer able to control the order of the listeners relative to the reporter. In the reporter names list, the order of the strings in the list controlled the order of output on assertion failure. For example, I put "console" first, and my reporter second, so that my output came after catch's output. With listeners, I cannot control this.

Having no control over this seems fine, depending on why listeners were designed and the problems they're intended to solve. It could be that they were never intended to be for console output. In this case, I expect no change. But the problem still remains: How do you amend console output in a controlled way? If reporters can't do this (because you can only have one) and listeners aren't for this, then there's no solution. Either this is solvable today and just a documentation problem, or there's some serious design confusion.

@horenmar
Copy link
Member

Closed in favour of #1712

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants