DataTestMethod with Enumerators fail in NCrunch

Kaerwek

#10928 9 years ago 07 Aug 2017 08:05 UTC

We're using DataTestMethods with enumerators, e.g.

[DataTestMethod]
[DataRow(AlignmentX.Center, "middle", DisplayName = "Returns \"middle\" for given AlignmentX = Center")]
[DataRow(AlignmentX.Left, "start", DisplayName = "Returns \"start\" for given AlignmentX = Left")]
[DataRow(AlignmentX.Right, "end", DisplayName = "Returns \"end\" for given AlignmentX = Right")]
public void Test(object input, string expected)

The tests fail because NCrunch converts the chosen enumerator to the backend value. When entering a breakpoint in debugger mode on first line, I see that input is an integer, 2, in the first case.

If I had a test that only tested for AlignmentX, I could perhaps code past it, but currently I have loads of other datarows and datatypes. Forcing everyone in the organization (since all dont use NCrunch), to create specific tests for DataTestMethods that use a specific enumerator isn't really feasible either.

Is this a known issue?

Remco NCrunch Developer

#10929 9 years ago 07 Aug 2017 08:09 UTC

Hi,

Thanks for sharing this issue.

Until now, I was not aware of this issue, but it does make sense that it could exist. There is some complexity in the handling of DataRow parameters, and you've managed to find an alignment hole.

I'll see what I can do about getting this fixed in the next build.

Remco NCrunch Developer

#11103 9 years ago 31 Aug 2017 01:32 UTC

I've taken a deeper look into this issue, and prepared a fix expected to be in a new version of NCrunch released by early next week.

The fix is narrow in scope. It will only work when MSTest is set to use a framework utilisation type of 'Dynamic Analysis'. This can be adjusted in your NCrunch configuration.

For performance reasons, the default mode of operation under MSTest is set to 'Static Analysis'. Unfortunately, static analysis does have some compatibility trade-offs that can affect edge cases like this one. If you are in a situation where other team members are using different test runners to NCrunch and alignment between runners is critically important, I recommend using Dynamic Analysis instead.

Kaerwek

#11454 8 years ago 07 Nov 2017 07:00 UTC

Hi Remco,

After seeing your reply since two months back, about a fix to this issue being included in a new version in the patch, I was hopeful that I'd find a patch or version release somewhere by now.

I have however 3.11.0.9, which seems to be latest. Is there a patch somewhere I can download and fix this issue with?

Remco NCrunch Developer

#11455 8 years ago 07 Nov 2017 07:34 UTC

Kaerwek

#11456 8 years ago 07 Nov 2017 08:05 UTC

Thanks Remco,

I Unblocked the nCrunch.VSIntegration2017.dll file, in order to get Visual Studio to load NCrunch properly without causing an exception.

However, I'm seeing this for every project in the NCrunch Tests tab:

nCrunch.TaskRunner.Ipc.IpcConnectionClosedException: The connection has been closed
at nCrunch.Core.ProcessManagement.ExternalProcessManager.(ProcessorArchitecture , ProcessLoadParameters )
at nCrunch.Core.ProcessManagement.ExternalProcessManager.LoadExternalProcess(ProcessLoadParameters parameters, GridClientId client)
at nCrunch.Core.BuildManagement.BuildProcessLauncher..()
at nCrunch.Common.PerformanceTracking.PerfTracker.TryTrackActivity(String name, Action activity)
at nCrunch.Core.BuildManagement.BuildProcessLauncher.(Action`1 , ProcessorArchitecture , GridClientId , BuildSystemParameters , IList`1 )
at nCrunch.Core.BuildManagement.BuildProcessLauncher.AnalyseComponentBuildInExternalProcess(ComponentLoadParameters parameters, IList`1 customEnvironmentVariables)
at nCrunch.Client.ComponentLoader.SnapshotComponentLoader.(ProcessorArchitecture , String )
at nCrunch.Client.ComponentLoader.SnapshotComponentLoader.CreateComponentFromXml(FilePath projectFilePath, ParsedBuildXml projectXml, FilePath solutionFilePath, String[] additionalFilesToIncludeAtSolutionLevel, Boolean isLoadedFromFile, VisualStudioVersion vsVersion, ComponentUniqueName componentName, TaskSettings componentTaskSettings, Exception parseException, String targetFramework)

Is there anything I can do to get NCrunch running with this build, or is it unstable?

Kaerwek

#11457 8 years ago 07 Nov 2017 08:06 UTC

Oh, and I only grabbed what I thought I needed, the NCrunch_VS2017_3.12.0.4.zip.

Remco NCrunch Developer

#11458 8 years ago 07 Nov 2017 08:22 UTC

I recommend installing via the MSI rather than the ZIP, if possible.

My first guess would be that the O/S is blocking NCrunch from running its task runners. If you need to install the ZIP, check that the files aren't being blocked. See here for specifics: http://www.ncrunch.net/documentation/manual-installation-instructions.

Kaerwek

#11459 8 years ago 07 Nov 2017 09:00 UTC

Hi,

Got it installed and working, however the issue, Datarow with enums don't work still.

I confirmed that I'm running now 3.12.0.4. But I still see these failing tests. And entering debugger mode, I see the integer as input still.

//
// Summary:
// Svg vector effect
public enum VectorEffect
{
//
// Summary:
// No effect
None = 0,
//
// Summary:
// Non scaling stroke
NonScalingStroke = 1,
//
// Summary:
// Inherit
Inherit = 2
}

Best regards,
Kaerwek

Deleted post

#11460 8 years ago 07 Nov 2017 09:02 UTC

This post has been deleted.

Kaerwek

#11461 8 years ago 07 Nov 2017 09:02 UTC

It's later compared against this in a switch statement, and as it's passed as an pure integer, it fails the switch case:

case VectorEffect x: return $"'{x.GetString()}'";

Remco NCrunch Developer

#11462 8 years ago 07 Nov 2017 09:22 UTC

Can you confirm whether you set your framework utilisation type for MSTest to DynamicAnalysis? As described earlier, it just wasn't feasible to support all use cases of the DataRow under StaticAnalysis.

Kaerwek

#11640 8 years ago 14 Dec 2017 08:32 UTC

Just installed newest version, 3.12.0.15, and can confirm that by having UseDynamicAnalysis, the tests no longer fail.

Thanks Remco!

Remco NCrunch Developer

#11641 8 years ago 14 Dec 2017 10:08 UTC

Great! Thanks for confirming!

Post a reply