Add new result writers about ward

[darrenburns]

As mentioned in #130 @DorianCzichotzki, I'd be more than willing to accept a PR that added something like an --output-json or something similar (happy to discuss the API).

[onlyanegg]

This is what I'm thinking for result writers. We could have a different class for each type.

ward
`-- writers
    |-- details.py
    |-- dots_global.py
    |-- dots_module.py
    |-- duration.py
    |-- errors.py
    |-- header.py
    |-- json.py
    |-- summary.py
    `-- utils.py

Each writer would have a super simple API. Most writers don't need to be passed the suite; instead the write method just takes the results.

class AbstractWriter(ABC):
  @abstractmethod
  def write(self, results: Iterable[TestResult]) -> None:
    '''
    '''

Then we could have aliases for output style and a factory function to create writers (maybe in writers/__init__.py).

test_per_line = [DetailsWriter, ErrorsWriter, SummaryWriter]
dots_module = [DotsModuleWriter, ErrorsWriter, SummaryWriter]
dots_global = [DotsGlobalWriter, ErrorsWriter, SummaryWriter]
json = [JsonWriter]
default = test_per_line

def get_writer(writer: str) -> AbstractWriter:
  return writer_dict[writer]()

The config / argument could then reference these options, and could be customized.

ward:
  output:
    - errors
    - summary

--output[test-per-line | dots-module | dots-global | json]
-o errors summary

Then in run.py, the API would look something like this.

# run.py
def run():
  ...
  results = suite.run()
  writers = [get_writer() for writer in writers]
  for writer in writers:
    results = writer.write(results)
  ...

Benefits

• This gives the user a lot of flexibility with output
• Creates an easy extension mechanism for the future

Problems

• Maybe too much flexibility? It allows the user to create nonsense outputs
• Currently the tests variable is a generator, which is a good design decision, because it allows you to display test results as they are available. In this method we would have to store the results of the tests, at least after the first writer runs.
  • Maybe we could have a wrapper that stores the results
  • Maybe we could have each writer return a list of results
    • I think I like the pipeline method better; it seems simpler - fewer classes, less code - and it keeps the API more similar to what it currently is

Questions

How do we deal with the Header?

It needs Suite metadata like time to collect and number of tests (possibly more in the future). This info is not contained in the test results, but we'd like it to be available for the JSONWriter (maybe others in the future). This is an open question for me since it doesn't fit in with the API I've described above.

• Maybe we pass in the suite instead of the TestResults?
• Maybe the JSON writer has a slightly different API? Maybe it takes the suite as well as the TestResults?
• Maybe we have a separate metadata writer that takes anything it needs. For JSON, the result of JSONMetaDataWriter could be merged with the result of JSONResultWriter

# JSONMetaDataWriter output
{
  "metadata": {
    "num_tests": 8,
    "collected_in": 0.1
  }
}

# JSONResultWriter output
{
  "results": [
    ...
  ]
}

[darrenburns]

Thanks for the detailed write-up! I've read it but will need some time to think about it in detail, and I'll get back to you when I've done that.

[darrenburns]

@onlyanegg Thanks again for this write-up and sorry for the delay. I've been low on open-source motivation™️ recently 😆

I think this approach might be adding too much complexity for a level of flexibility that isn't necessary for the vast majority of users. For example, I'm not sure I see benefit in breaking out Details writer into its own class. I do like elements of it though.

Right now, the SimpleTestResultWrite extends an abstract TestResultWriterBase. This base class defines all the methods that need to be implemented to output a full set of results. Right now though, it's not really independent of where that output is written to. It assumes the output is going to the terminal (a bad assumption). I think my preferred approach would be to make TestResultWriterBase truly independent of where the output is being sent to, rename the SimpleTestResultWrite to TerminalResultWriter (or something like that), and have another subclass of TestResultWriterBase called JsonResultWriter. So it would stick to and extend the existing approach, but not split things out to extent of your proposal.

--output[test-per-line | dots-module | dots-global | json]

I don't know if I'm understanding this correctly, but this would make it impossible to handle the situation where you wish to output to the terminal and output to a JSON file. My suggestion on this:

Keep the existing test-output-style and add an --output-json=[FILENAME] etc. option (my favoured approach). You could then output to several files if needed (for example output to TAP/XML for use in a CI system, and output to JSON for further processing elsewhere).

--test-output-style=dots-module --output-json="results.json" --output-xml="results.xml"

The example above would output to the terminal, and at the same time write a results.json and a results.xml file.

Currently the tests variable is a generator

To solve this, I think we could extend the pipeline approach already in use: yield each result as it's generated from each writer. Then the next writer in the pipeline will be able to use that result as it becomes available. This would involve some minor restructuring of how the writers behave, but I don't think it would be a great deal.

So in run.py we'd instantiate all the writers required, and pass the results through them (using the pipeline approach).

As for the structure of the JSON file itself, I'd like to try and keep it relatively consistent with a on-disk caching element I'd like to add to Ward. My initial thinking on this is to make that cache a JSON file, which would store the results of previous tests. Seems like something that could definitely be re-used here 🤔. Initial thoughts on this are to essentially output vars(test_result) as JSON for each TestResult, and have a metadata section, as you suggested in your post.

[onlyanegg]

Thanks for replying, @darrenburns :)

--output[test-per-line | dots-module | dots-global | json]

Sorry, that was confusing. I meant for this to be a multiple input, like --output test-per-line errors or --output test-per-line --output errors. In that way, any combination of multiple output writers could be used.

Keep the existing test-output-style and add an --output-json=[FILENAME]

I think the target and the format are getting conflated here. You may want to send JSON output to the terminal. We could have multiple targets to send the results to (eg. terminal, file, databases, email, etc.) and multiple formats to format the test result output (eg. details, dots_global, dots_module, json, etc.). While we don't need to support every combination of these (eg. why would you want to send dots to a database) I don't see the harm in making an API general enough to support it (with aliases for commonly used combinations). We don't need to implement everything at once, but I think it makes sense to create an API that is easily extendable to support use cases that we're currently unaware of.

To solve this, I think we could extend the pipeline approach already in use: yield each result as it's generated from each writer.

One issue I see with this is that results from each writer would be interleaved which could be confusing. Maybe it wouldn't be an issue in your version of this API.

As for the structure of the JSON file itself, I'd like to try and keep it relatively consistent with a on-disk caching element I'd like to add to Ward

I fully agree with this, and I don't see any reason why it should be any different at all.

Initial thoughts on this are to essentially output vars(test_result) as JSON for each TestResult, and have a metadata section, as you suggested in your post.

I think it might be worthwhile to create a legit JSON encoder or a .to_json() method, because the cache seems like it'll be a fundamental part of Ward, and vars can be kinda messy (ie. it has private attributes and methods).

[darrenburns]

I meant for this to be a multiple input

That makes more sense now, thanks 😄

I think the target and the format are getting conflated here.

I agree to an extent. Sending JSON to the terminal would probably be achieved in that case by --output-json=stdout. We would probably want to ensure that only a single output type is sent to stdout, regardless of which approach we take.

--output test-per-line --output errors

This seems like a conflation to me too, it's a single option which is defining which output mode to use, and which sections to show -- if I'm following correctly?

Another thought I had was --output <type>:<target>, so for example --output dots-module:stdout --output json:results.json --output xml:results.xml. With this approach, users could send JSON to stdout using --output json:stdout and pipe this output into other programs for further processing.

results from each writer would be interleaved

I'm thinking that each writer has to write to a unique source for a single run. You can't have 2 writers writing to stdout at the same time, or 2 writers writing to a file of the same name at the same time. Ward could enforce this restriction, which I think is sensible (let me know if you can think of a use case to the contrary). If you're outputting JSON to the terminal, it's unlikely you also want to output the standard terminal output. You probably want it written to stdout so you can pipe it elsewhere.

I think it might be worthwhile to create a legit JSON encoder

Yup agreed. I wasn't suggesting we use vars directly, I was just suggesting that the output should be roughly equivalent to taking vars(test_result) and making it JSON. :)

[darrenburns]

--output test-per-line --output errors

Just realised I wasn't following this. For customising the output, I think I'd rather give the option to the users to show/hide specific sections using separate options rather than using the output option for all of that.

[onlyanegg]

I see details, dots_global, dots_module, duration, errors, summary as being different ways to format results, but ways that you may want to combine on a single target (eg. the SimpleResultTestWriter combines details, duration, errors, and summary). Having a writer (formatter?) for each one means that users can precisely define how they want to format results. For instance, in another issue, I think I mentioned wanting to be able to display the summary only and then being able to go back and dig into the errors if there are any (a use case for the cache). This use case could be easily handled by setting a config parameter to only use the summary writer.

I think you're totally right though that the more general an API is the larger the surface area that could hide bugs. And maybe it's not worth it since the majority of people will not use it like that. But I the software I often like best is software that is simple to use (read: has great defaults for the majority of use cases), but can also be bent to my will.

--output test-per-line --output errors

I think your issue with this is explained above

We would probably want to ensure that only a single output type is sent to stdout, regardless of which approach we take.

I tried to explain why I don't agree with this above

Another thought I had was --output <type>:<target>

I think this is a really nice looking format; however, it wouldn't work in my proposal since we would need to specify multiple types. More generally, we could do something like --format details errors summary --target terminal

[darrenburns]

I tried to explain why I don't agree with this above

I think you're using your own idea of what e.g. dots-module would do given your proposal rather than what it currently does (it currently includes error output, summary, etc). In my example dots-module means include everything that it currently does, and that's why I don't see any reason to have more than one of those being sent to stdout.

I'm struggling with the generality of it and found it quite confusing (as you can probably tell 😆). It is very flexible, but I don't know that I find it to be the best way to choose which information you want to be included in the output.

To try and give more insight into why I'm struggling with it (I apologise if I'm still not understanding you fully here): My view is that Ward should only provide options for customising the terminal output. Ward will have sensible (for the majority of use-cases) defaults, and if you want to hide or show sections, it can provide options for doing so.

It should provide options for customising the terminal output only, in my opinion, because it outputs to the terminal in an unstructured format that we can't expect external tools to parse. If you'd like to show the slowest test durations in the terminal use --show-slowest (thanks!). If you want to hide all PASSing tests and only show fails in the terminal, use --hide-passes (doesn't exist, but it's an example).

For structured output (e.g. JSON), I don't believe Ward should be providing a means of customising what to output. Structured output is machine readable and is designed to be parsed by external programs, and I'd argue that if the user wishes to filter that JSON they can do a much better job using a command line utility built for the job (e.g. jq):

ward --output-json stdout | jq "{expr to filter passing tests}" > failing_tests.json

The example above would output JSON to stdout, and jq could be used to remove all passing tests. failing_tests.json would contain only what is needed due to our filter. jq is going to be infinitely more flexible than Ward could ever be at providing this functionality. With this JSON being sent to stdout, I see no reason why we'd also want to send dots-module or XML to stdout.

The idea is that Ward provides the structured output which conforms to a well-defined schema, and the user can use a tool specifically designed for the job to filter/format it as they require, as per the Unix philosophy.

Hopefully that explains my concerns, and hopefully I'm not totally off the mark. I'd love to hear some feedback from others on this. API design is obviously subjective and I'd love to hear input from others here :)

[onlyanegg]

You're right. There's really no point in sending dots and json to the terminal at the same time or many other combinations for that matter.

If I can try to summarize your proposal:

We'd have a number of different formats: human, json, xml, etc, and, independently, a number of different targets: terminal, file, etc. The human format would encompass all of the current output modes: dots, details, summary, slowest, and we could configure that class to include or exclude certain sections. For structured formats, we likely wouldn't include any type of filtering. Is that right?

Upon reflection, I think that's totally reasonable, and I think it does make it less complex. However, I also think it makes it a bit harder to extend the human format.

Anyway, I appreciate you hashing this out with me today :)

[darrenburns]

Yes - I think we're on the roughly same page now. I've been thinking more about this today and I think my favourite approach is the one I'll describe below. I'll describe it with examples because I think that's probably an easier way to get my idea across than trying to describe what different options do.

The general format:

ward --output <format>:<destination>

We'd have several formats (the first 3 of these already exist):

• test-per-line
• dots-global
• dots-module
• json
• xml

And destination can be:

• stdout - to send the formatted data to stdout
• A path to a file - to send that formatted data to a file.

Specific examples

The two examples below would be functionally equivalent, and behave exactly the same way as ward behaves when you run it with no args right now:

ward

ward --output test-per-line:stdout

If we wanted to output to our terminal using the current global "dots" output, and write the results to a JSON file at the same time, we could do:

ward --output dots-global:stdout \
     --output json:results.json \

I'd suggest that by specifying an output without the part after the :, stdout is used by default. So the above example could be written as:

ward --output dots-global \
     --output json:results.json \

If we wanted to only send JSON to the terminal, so we could pipe it into another tool or POST it to and endpoint:

ward --output json    // same as 'json:stdout'

Invalid invocations

Each destination has to be unique in a single invocation. We can't write to stdout twice for example. The following examples would be illegal:

ward --output json \
     --output xml

That's two outputs targeting stdout (which would cause the interleaving problem you mentioned earlier, so we fail fast here).

We can't have two different formats with the same file destination either.

ward --output json:results.out \
     --output xml:results.out

That's illegal because they both have the same destination.

To add new human readable formats, you just extend the already existing base class and implement the methods you want.