Welcome Guest! To enable all features please Login or Register.

Notification

Icon
Error

Tests not marked as 'impacted' when static fields change
Tim Long
#1 Posted : Sunday, May 24, 2020 11:52:28 AM(UTC)
Rank: Member

Groups: Registered
Joined: 8/20/2016(UTC)
Posts: 17
Location: United Kingdom

Was thanked: 1 time(s) in 1 post(s)
I use MSpec as my test framework. Consider the following example:

Quote:

[Subject(typeof(SimulatorStateMachine), "electroluminescent panel")]
internal class when_the_brightness_is_set_nonzero : with_simulator_context
{
Establish context = () => Context = ContextBuilder.WithOpenChannel().InClosedState()
.WithPropertyChangeNotifications((sender, args) => PropertyChanges.Add(args.PropertyName))
.Build();
Because of = () => Context.Simulator.SetLampBrightness(128);
It should_change_the_expected_properties = () => PropertyChanges.ShouldEqual(ExpectedPropertyChanges);
It should_not_change_state = () => Context.StateChanges.ShouldBeEmpty();
It should_not_turn_the_lamp_on = () => Context.Simulator.LampOn.ShouldBeFalse();
static List<string> PropertyChanges = new List<string>();
static List<string> ExpectedPropertyChanges = new List<string> { "LampBrightness" };
}


The very last line of that test is a static field. In the editor, this line displays as a white dot, indicating that it does not impact any tests. If I change the value in quotes, the test does not get marked as 'impacted' and does not run.

It seems self-evident to me that editing a test in any way impacts the test, and therefore this is something that should cause the test to run. Is this behaviour intentional? Is there a way to change it?

Best regards,
Tim
Remco
#2 Posted : Sunday, May 24, 2020 1:04:00 PM(UTC)
Rank: NCrunch Developer

Groups: Administrators
Joined: 4/16/2011(UTC)
Posts: 6,101

Thanks: 792 times
Was thanked: 1039 time(s) in 989 post(s)
Hi Tim,

Static initialisers are a difficult one to catch with NCrunch's code coverage tracking (and therefore impact detection). This is because of the manner in which the CLR executes them.

Static fields are generally initialised the first time the containing class is accessed during runtime, through the class static constructor. This access can happen due to related code being executed, or perhaps through reflection. It can happen well before the test is actually intended to be executed. Because NCrunch's code coverage system only marks out lines that are physically executed by the CPU after the test has marked its execution as started, there is a high chance that these lines won't be included in the test because they were actually executed earlier in the run. There is also the potential for inconsistent behaviour here, as it's possible for the fields to be initialised during the run of an entirely different test that might not be related to target code. Remember that NCrunch will re-use test processes to execute multiple tests where possible, and a class static constructor can only execute once per application domain.

The core issue here is actually a matter of perspective. Logically, the code is covered by the test. The behaviour of the test depends on it. However, physically, the code is not actually executed during the test. If you stepped through your code with a debugger, the code would not be touched with any of the test entry methods in the call stack.

NCrunch is only capable of tracking physical code coverage. To make the jump to tracking logical coverage, we would need to conduct a very complex and very expensive analysis of your code to establish everything it depends on, and probably hook this up with some kind of probability system to handle things like events tied to externals or O/S callbacks deep inside the system. Such an analysis would eliminate the advantages of impact detection, as it would take so long that you might as well just run all the tests anyway. Thus we end up in a situation where impact detection can never be perfect, we just hope it's good enough to be useful most of the time. The situation you've uncovered here isn't something that we're able to efficiently cover with our existing impact detection system, so I would recommend using the 'Run all tests automatically' engine mode in this scenario instead.
Tim Long
#3 Posted : Sunday, May 24, 2020 9:22:29 PM(UTC)
Rank: Member

Groups: Registered
Joined: 8/20/2016(UTC)
Posts: 17
Location: United Kingdom

Was thanked: 1 time(s) in 1 post(s)
I guessed you would say something like that, so no surprises. It's just a bit annoying with MSpec in particular because all fields are required to be static, so it is a much bigger problem than for other frameworks.

Could you not implement something specific to MSpec? Perhaps a Roslyn hook that watches the syntax tree and detects when a test context class changes? A context class is defined as any class having a member field of type Machine.Specifications.It

Regards,
Tim
Remco
#4 Posted : Sunday, May 24, 2020 11:40:02 PM(UTC)
Rank: NCrunch Developer

Groups: Administrators
Joined: 4/16/2011(UTC)
Posts: 6,101

Thanks: 792 times
Was thanked: 1039 time(s) in 989 post(s)
I'll take a closer look, but can't make promises. It would be tragic if the solution to this increased all instrumentation times by 20% or something.

Could you perhaps use a property instead?
Tim Long
#5 Posted : Monday, May 25, 2020 3:17:40 AM(UTC)
Rank: Member

Groups: Registered
Joined: 8/20/2016(UTC)
Posts: 17
Location: United Kingdom

Was thanked: 1 time(s) in 1 post(s)
Remco;14734 wrote:
Could you perhaps use a property instead?


I could certainly use a property, but it would still have to be static because of the way MSpec works. So will it make any difference? I'll give it a go tomorrow as I'm just off to bed.

--Tim
Remco
#6 Posted : Monday, May 25, 2020 4:07:30 AM(UTC)
Rank: NCrunch Developer

Groups: Administrators
Joined: 4/16/2011(UTC)
Posts: 6,101

Thanks: 792 times
Was thanked: 1039 time(s) in 989 post(s)
A static property would be very different in terms of coverage tracking and impact detection, because its value would be initialised and assigned when the property is called, instead of in the class static constructor. If you use a property, the coverage and impact detection hole is gone. Actually, it's probably better practice too, because it means that if there is an exception thrown when the value is initialised, you'll catch it during the test instead of inside some unknown routine where the exception reporting is less clear.
Users browsing this topic
Guest
Forum Jump  
You cannot post new topics in this forum.
You cannot reply to topics in this forum.
You cannot delete your posts in this forum.
You cannot edit your posts in this forum.
You cannot create polls in this forum.
You cannot vote in polls in this forum.

YAF | YAF © 2003-2011, Yet Another Forum.NET
This page was generated in 0.047 seconds.