Adding JUnit support

[b4nst]

Refactoring of PR #192

As requested, I've worked on branch refactor-execution. I've adapted the code to use the MacroCallbacks new implementation.

I've not added the --junit option yet, we need to discuss on how to pass config parameters (working dir, filename...) to MacroCallbacks during execution. Currently output file is written in execution_dir/test-results.xml by default.

I'm also not totally sure if this is the behaviour expected.

Please tell me if I misunderstood something.

Cheers !
Bastien.

[svanoort-jenkins]

Can one of the admins verify this patch?

[svanoort]

@bastienar Having skimmed through this now, I think this restructuring is right along the behaviour I was looking for! It's cleaner, it lets us test the JUnit bits independently, and it means we don't have to intermix logging with execution logic.

One thing to change: you used the start_macro and end_macro callbacks within the run_testset method to mark its beginning and end, and start_test_case for individual tests: A Macro is a test (or benchmark, or parameterized testcase, eventually maybe a load test). The start/end macro callbacks are for beginning and end of a test, not a group. You'll want to create a start_testset & end_testset method in your JUnitCallbacks that handles this. I will add the appropriate methods to the main Callbacks (and we can add to the execute_testset method).

Here's how we handle passing config information in to pick/configure the callbacks bundle: we create the Callbacks object here: https://github.com/svanoort/pyresttest/pull/171/files#diff-72237f18d508a05bb8c65dc41c4c0de5R189 - we can look at the JUnit config option and if present use that instead of the logger. The Callbacks can be initialized with file path, for example and store it in the instance. The logic to read configuration and pick/configure the callbacks should be isolated into a separate method call though.

For the file read/write we'll probably need to add a start_output and end_output callback (the latter called before exit) to cap off the XML file. The easiest way to do the latter (a bit hacky, but I don't want to block you while I rearchitect around it) is to use a destructor on the Callbacks instance.

One other smaller thing to do in the future: don't add extra whitespace changes. I'm not going to ask you change it on existing commits because I don't wish to be That Nitpicky Open Source Reviewer. But, extra whitespace makes it harder to skim the diff, and in cases like this where I'm already going to have to do some messy merge conflict resolution, it can make the problem harder.

Stripping trailing whitespace & PEP8 autoformatting are easily done periodically on master branch with regexes and automated tools (I've done this in the past).

I haven't looked closely at the inner logic of the callbacks closely yet (I am assuming it's following the mappings in #43 more or less).

[b4nst]

So junit config option will be in the -- config part of the testset file and not in the command line arg, right ?

Also I'm not following the #43 as I should yet, because I've just re-factored my old code, which were doing this mapping :

TestSet <--> TestSuites in Junit
Group <--> TestSuite in JUnit
Test <--> TestCase in JUnit

I'm aware that's not correct, and I will update that to match the #43 mapping, with maybe adding group in the classname property of the test case (as classname='groupName.testName')

Also sorry for the trailing whitespace, I didn't pay enought attention.

[b4nst]

There is an issue following this sort of mapping if we get the junit config from --config in the testset file, because we could get multiple config possibilities in the TestSets list.

So I suppose I've misunderstood and junit config option should stay in the command line args.

[svanoort]

@bastienar

So junit config option will be in the -- config part of the testset file and not in the command line arg, right ?

No, command line only is fine for now, I think it's dangerous to allow creating different JUnit outputs for each testset right now (too many edge cases). But by making the callbacks somehing passed into the macro execution and the testset run method, we can allow for ways to do this later. Also, the whole way we handle config is getting massive changes in the near future anyway with #101 and some of the associated stories. Basically: eventually we can do it, but would rather get the basic case covered (plus the pretty ambitious refactors I'm doing) before we try to get fancy with that.

maybe adding group in the classname property of the test case (as classname='groupName.testName')

I think that's reasonable for now (setting it as a property might make sense too). You make a good point here about group vs. testset -- I hadn't started the implementation on JUnit output yet, so it's possible we might come with a better way to do this with the JUnit output.

At the end of the day though, this is trying to map together two different models of a test system, so there will probably be some leaky abstractions and impedence mismatches. It doesn't have to be perfect to be useful to people.

I'm going to be tied up until this Saturday or Sunday, EST though, so hopefully this gives enough to work on for now? We do need tests too, so with that in the mix hopefully you've got enough here that you will not be blocked.

[BGarretson]

Where are owners at with this change? This would be really helpful to have

[mvitiuk]

Hi guys, this feature will be very helpful, can somebody look at it? It's a year soon since open date.

[lennartblom]

What's the status on this feature? Would be very helpful if someone of the project could response to this nice content! 👍

[samirsss]

Can we get this feature merged, since i'd like to use this in our CI systems too.

[alexjomin]

Hello @svanoort, pyresttest is awesome but we really need this feature !