vipm-io / caraya

Assertion and unit test framework for LabVIEW
Other
59 stars 33 forks source link

Caraya Report contains duplicates of every assert regardless of hierarchy #124

Closed tannerblair closed 3 years ago

tannerblair commented 3 years ago

When I call test VIs in a loop, Caraya seems to get confused about which parent a given assert refers to. In the image, the tree is correct. The JUnit XML contains a ton of duplicate entries

image

image

tannerblair commented 3 years ago

Looks like it's showing up in reports, but it's actually storing it in the results variant:

image

This would go a long way toward explaining #56 since the cost would likely be on insert and this is storing exponentially more results than required.

francois-normandin commented 3 years ago

Can you provide a simple test code that shows this effect? I'm particularly interested in how you call the assertions in the loop and how your tests are labeled.

If there are exponentially increasing duplicates, this is a new way of working the node and naming them. It will be useful to have a solid use case in order to find a workaround at the source, like you suggest, rather than filtering them in post processing.

tannerblair commented 3 years ago

@francois-normandin , sorry for the complicated example. I just sanitized the code I was working on at the moment. There is actually a unit test in the PR that is the base case, but I can make a quick example in a bit and send it to you.

After debugging, the issue is how you are matching Test Events to Test Registrations. You are using the call chain, which is identical for all calls in a loop, meaning that every call matches every caller. The PR I submitted changes this to use the Test ID, which is actually unique.

tannerblair commented 3 years ago

Here's the base case, no loop required. Duplicate Reporting in Caraya.zip

I'll save you a click with some screenshots though:

Outer Test VI: image

Inner Test VI: image

Resulting Test Report: image

tannerblair commented 3 years ago

Here's the same VI run in with the code in my PR: image

What I changed:

Basic Test Manager.lvclass:Generate Test Results.vi: image

New Method Basic Test Manager.lvclass:Get Test Events by Test ID.vi: image

francois-normandin commented 3 years ago

@tannerblair great, thanks. When XML was added to the project, there were only assert IDs. We later added test IDs to solve a duplicate assertions name issue across multiple tests. The XML report was not changed to match. I'll integrate your tests into the automated suite. Nice to know we'll have something to catch this on the future.

francois-normandin commented 3 years ago

@tannerblair confirmed, and I don't see any impacts on the rest of the framework's tests. Funnily enough, the Get Tests Events by Key was already there... it turns out that the storage key is TestID, so no need for an extra node after all.

image

Thanks for fishing this out. We'll have a fix for release 1.2.0.

On another note... it does not significantly change the processing time. That will be a separate issue to fix.

tannerblair commented 3 years ago

Great, thanks!

francois-normandin commented 3 years ago

Included in 1.2.0 to be released soon https://github.com/JKISoftware/Caraya/milestone/6