Did the detector fail to notice a non-attribute change during your stream? Sorry I tried to check through the links you gave me but I wasn't able to notice it .. sometimes it isn't very obvious (I guess this is part of the problem).
NCrunch's behaviour around attributes is to consider them a part of the method. This means that if you have a method that is covered by tests and you add an attribute to it, all tests that cover the method will be considered impacted. So basically the attributes form a part of the method's hash signature and they are included in the detection.
The above approach doesn't work for situations where you have a method or attribute that is not covered by tests (i.e. black markers), and you add an attribute to it which is used as a marker by other code to dynamically execute it. For example, consider the following code:
Code:
foreach (var type in GetType().Assembly.GetTypes())
{
var instance = Activator.CreateInstance(type);
foreach (var requiredProperty in type.GetProperties().Where(m => m.CustomAttributes.Any(a => a.AttributeType.Name == "Required")))
{
var propertyValue = requiredProperty.GetGetMethod().Invoke(instance, null);
// Do validation on propertyValue here
}
}
In concept, this is probably quite similar to the validation code used during your live stream. This code uses reflection to find properties that implement a specific attribute. The impact detection rule required to handle this scenario is such where we need to detect when a user has added the 'Required' attribute to a property, then all tests making use of the entire validation framework are considered impacted and must be re-run.
Consider that in this scenario, simply searching for covered code around the new attribute isn't enough. This is because when the attribute is first being added to the property, the property isn't covered at all, so we have no baseline data to tell us which tests should be considered. When you remove the attribute from the property, we likely will have coverage in place and we will probably detect the change.
It's easy to think of impact detection as being a process of choosing which tests are likely to be affected by a change made to your code, but actually the better way to think of it is as a way of excluding the tests that you know haven't been affected. Given a full suite of tests and a set of code changes, how can you tell which tests can be safely excluded from execution? Once tests start making use of reflection to determine which areas of code they interact with, the process of exclusion becomes very difficult because our exclusion rules become coupled to the logic around how reflection is used.
We could, for example, decide that any code calling into Assembly.GetTypes() could be considered as 'covering' the surface all types in the given assembly, but then every time we make a structural change to any type in the assembly all tests touching this code will be impacted, so we'll have a large number of tests running all the time and the process of exclusion wouldn't work well at all.
We then have further extensions of this, such as how to handle this kind of code when it appears in 3rd party assemblies, or even in the runtime itself.
This is also not the only situation where reliability becomes a problem for impact detection. Consider the following code:
Code:
[TestClass]
public class UnitTest1
{
private static string _myStringValue;
private static string LazyLoadValue()
{
// This code is only run once per test process
return MyDatabase.LoadStringFromDatabase();
}
private static string MyString
{
get
{
if (_myStringValue == null)
_myStringValue = LazyLoadValue();
return _myStringValue;
}
}
[TestMethod]
public void TestMethod1()
{
Assert.That(MyString.StartsWith("123"));
}
[TestMethod]
public void TestMethod2()
{
Assert.That(MyString.EndsWith("abc"));
}
}
The above class contains a method (LazyLoadValue) that both tests logically depend on, but most likely will only ever be 'covered' by one of them. This is because both tests probably execute quite quickly and will likely be run by NCrunch in the same physical process.
This sort of situation presents a problem for impact detection, because as far as NCrunch is concerned, the only test actually dependent on LazyLoadValue is the first one to use it inside the test process. Since both tests have different pass/fail criteria for what they receive from this method, it's possible for you to make a change to LazyLoadValue that should trigger a test failure but doesn't because NCrunch only runs one of the two tests automatically.
There are ways to handle this sort of situation. For example, instead of using code coverage to determine which tests might be impacted, we could use structural data from the test instead. We would simply look at each of our test methods, see which methods they call, then form a large dependency tree for each test based on what it might call without any consideration for tracked coverage. The problem with this approach is that it's extremely broad. If you change even the smallest method down the bottom of your dependency tree, you'll likely impact all of your tests even though none of them are actually covering the code at all. This approach might still be better than 'Run all tests automatically' though, so we might add it some time in future as an extra option.
