TestSuite Timestamps Report All Constant about go-junit-report HOT 6 CLOSED

Thanks for the report.

Looks like the main problem is, as you pointed out, that the report is constructed only after everything has been parsed. Your suggested solution appears to work, but I think there's an alternative way to fix this that doesn't involve the added complexity of running report concurrently to parse and communicating via a channel. Instead, we can merge the report code into the parse function. I.e. move the report builder initialization before the for loop in parse and call rb.ProcessEvent at the same place where the events are added to the p.events slice.

Another potential problem I found is that the package timestamp in the report builder only gets set when we encounter a summary line (in CreatePackage), which is pretty much when the test has already completed. I'm not sure whether timestamp field was intended to contain the starting or ending timestamp of the testsuite, but I suspect it's the former. To fix this we'd have to modify the packageBuilder to store the creation timestamp in newPackageBuilder (which typically happens when we process the first event of a new package) and use that rather than the current timestamp in CreatePackage.

If you're interested in fixing these, feel free to send some PR's. Otherwise I'm happy to make the change once I have some time.

from go-junit-report.

Thanks! I'll take a look at implementing it as you describe, that makes sense as well.

I'm not sure whether timestamp field was intended to contain the starting or ending timestamp of the testsuite, but I suspect it's the former. To fix this we'd have to modify the packageBuilder to store the creation timestamp in newPackageBuilder (which typically happens when we process the first event of a new package) and use that rather than the current timestamp in CreatePackage.

I suspect it's meant to be the start timestamp as well. This wasn't impacting for me, but it probably makes sense to address at the same time. I was thinking we could also use the timestamp function minus the duration. I think if it was based on the first event of a package the reported timestamps per test could be well before the test actually started. I might be misunderstanding the fields though!

from go-junit-report.

Hey @jstemmer, sorry for the extreme delay/lack of comms. Anyways I finally took the time to take a closer look at implement the suggested solution. PR up and it works great!

from go-junit-report.

@jstemmer, sorry for the extra ping -- I just wanted to see if you may have an opportunity to review and merge the change.

from go-junit-report.

No problem. I've been busy lately, so didn't get a chance to take a look until just now. Thanks for taking the time to fix this!

from go-junit-report.

Thanks, much appreciated and totally understand being busy -- took me 5 months to actually make the change 🤷‍♂️

from go-junit-report.