banditcpp / bandit

Human-friendly unit testing for C++11
https://banditcpp.github.io/bandit/
Other
259 stars 37 forks source link

"before_each" misbehaves inside multiples "go_bandit" #164

Open m4c0 opened 3 years ago

m4c0 commented 3 years ago

Problem

I want to have a single executable with multiple go_bandit suites. Some of them may contain before_each.

Expected result

Each suite installed by go_bandit runs in a "sandboxed" environment and no variables or before_each would leak between them.

 Current result

A before_each inside a go_bandit works like a global, surviving longer than the lambda, triggering UBs.

Code example

This zip contains an example CPP. It declares two go_bandit and one of them initialises a unique_ptr in a before_each. Running as-is crashes due to a UB after the before_each was called in the wrong go_bandit.

Changing the order of the go_bandits causes Bandit to complain about before_each without an it.

This setup simulates what would happen if each go_bandit was in its own CPP file.

Request

I think Bandit needs to be modified to do one of these three possible solutions:

  1. Support multiple go_bandit with before_each. Maybe there's a way to treat go_bandit's lambda as a implicit describe?
  2. Explicitly deny a before_each inside a go_bandit. This seems to be the norm, because there's no mention to before_each inside go_bandit in docs. if so, then code should fail and docs needs to be explicit about it.
sbeyer commented 3 years ago

Thank you very much for your report. I'm going to look into this. My reply offhand is that I think to recall that every before_each/after_each needs a surrounding describe. When I first got aware of this, I had the feeling that go_bandit should get a mandatory string parameter which is used to describe the outer level.

m4c0 commented 3 years ago

Thanks for your prompt answer!

Both makes sense (go_bandit as s describe and *_each requiring a describe).

go_bandit with an extra parameter looks like a breaking change for users. Can we do a describe without a parameter? That would keep the API stable and sounds like a easy solution.

By the way, I kinda missed the description when I mixed two go_bandit in the same executable. Both info and spec yields a result that I can't identify the suite origin. That seems an extra argument to implement an implicit describe. 😀