-
Notifications
You must be signed in to change notification settings - Fork 106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Operation could destabilize the runtime #519
Comments
Can you please upload a repro project? |
It is internal project |
Perhaps try reducing the internal project, e.g. remove code so long as it still reproduces. |
We need some more information on this, the normal way is that you provide a similar working repro project. We dont need to see anything from your internal project, but we need to know what you are trying to do. (Thanks @siprbaum , you beat me to it :-) ) |
I reproduce it use empty nunit project packages.config
UnitTest1.cs
b7b3e470-73cd-11e8-ba0f-2dd83a1f0155.runsettings
run.bat
|
|
|
file b7b3e470-73cd-11e8-ba0f-2dd83a1f0155.runsettings is generated by VSTest task |
(MS also has noted the current behaviour as a bug, see links from @pvlakshm above. ) |
The UseVerifiableInstrumentation is set to false by the VsTest task when the platform is acquired via the vstestplatform tool installer (gets the nuget package) as the profiler may not be available in the GAC and needs to be resolved using certain environment variables set by the VsTest task. This is a must for code coverage to work when there is no VS installed on the target machine. This will have to be fixed otherwise code coverage will not work for NUnit tests when run using the VsTest Platform Nuget package (No VS). |
Thanks @ShreyasRmsft With "have to be fixed", do you mean in the VSTest task or will the project need its own runsettings file? |
I added custom runsettings with True </ UseVerifiableInstrumentation> |
All works fine with MSTest :( |
@OsirisTerje is the NUnit adapter throwing this exception? Is the adapter reading the data collector node in the runsettings? |
@ShreyasRmsft It is an exception coming from the NUnit Engine, so it is not the adapter itself, but the embedded NUnit engine. I am unsure about how this collector is intended to work, one of you guys did these changes earlier, and it seems that something here causes the engine to crash. And yes, the adapter reads the node and then sets some other properties: @NikitaEgorov Did you change the runsettings as I described above? (not only setting the flag to true, but replacing the DataCollector with InProcDataCollector? |
@OsirisTerje changing it to inproc is not the solution. Ideally in this case the engine should not be reading these settings at all as they are specific to code coverage. |
@ShreyasRmsft The engine don't read MS properties, only NUnit properties. The adapter does the conversion as shown above. Anything else being done must happen on the datacollector side. And this code above is from MS, so I assume you are setting whatever is needed correctly here ? |
@OsirisTerje I do not have a lot of context on the change that was done. But yes if the change that was made by one of us is trying to read this settings then it needs to be fixed. I haven't had time yet to see where exactly in the code this is happening. But Nunit should ideally ignore anything present in the code coverage node. Will get back to you on the fix. |
@ShreyasRmsft I believe the code involved were written by @navin22, i think this PR #391 was the essential one. I also see a later modification by @drognanar to "Use InProcDataCollector setting to enable sequential run", PR #510, but this is not released yet, it's for 3.11. |
Hi there, I have been told that this issue affects another issue (which I submitted). Thanks! |
@MoiMoiDG MS themselves are working on it on their side, but I'll check up on it. |
@OsirisTerje: Thanks! Is there any update on this? |
@ShreyasRmsft Can you mail me on what I need to do with the adapter to have this fixed? (terje at hermit.no) |
Since we use sourcelink ( and many more do the same), using the pdb’s as a indicator for the “dll’s under test” is likely to cause more such issues. And the test framework dll's should not in any case be part of the profiling. Do you call the profiler before or after you call the adapter? |
Is there any update on this issue? |
Sorry for such delay on this. Issue Explanation: Solutions: We are planning to include Shim dll as part of our XCopy package, & also we plan to release code coverage nugets, which will allow users to take explicit dependency on Shim dll. I'll update the thread once we have done either. For now only solution available to user would be use code coverage runsettings which will exclude NUnit dlls from getting instrumented. |
Hi @mayankbansal018, Thanks for explaining the problem!
Could you please provide an example of such runsettings file where it works? I know it is a workaround, but it would be helpful. Thanks! |
@MoiMoiDG the following runsettings should help you
|
Hi @mayankbansal018, Thank you! However, I still get the error in attachment. I attached the full solution too, for your convenience (it is just a small sample) ... :( |
@MoiMoiDG @mayankbansal018 : The project works when in Visual Studio, using the runsettings file, and failed without. After a bit hazzling it started working in both cases. Not really sure why. But in your log it says the runsettings ARE loaded, and I see you still get the same error. |
@OsirisTerje the caching of file issue can be only with Visual Studio, though we have not seen it too often, adding @AbhitejJohn, @ManishJayaswal to see if they have more info. @MoiMoiDG can you confirm if this ever fails for you in CLI when used with above runsettings? |
Hi everyone, Thank you for the support! @mayankbansal018: I am not sure that I understood your question. The log comes from a VM attached to TFS 2018.3 and "Test platform version = Installed by Tools Installer". |
The following seems to be related: microsoft/vstest#1950 |
Fixed with microsoft/vstest#1997, we have also released latest nuget package (16.0.2 preview) of test platform with above change, please upgrade to it. |
@mayankbansal018 Do you refer to the VSTest task or to which nuget package? |
@OsirisTerje I'm referring to nuget that gets installed via Test Platform nuget installer task in pipeline. If users facing this issue upgrade to latest preview, then it should be resolved for them |
The latest version of the console (16.0.2 preview) did not resolve the problem for me. |
Is there an update on this? Running 16.2.0 and still hitting this issue. We need code coverage from the nunit test results in order to achieve our targets. |
Please take a look at the comments in microsoft/vstest#1997. I have provided details on the fix. Make sure you have followed all the instructions and even after that you are facing issues please provide the full set of debug logs for the pipeline run and we will take a look. |
Seems to be fixed with the combination of: |
The text was updated successfully, but these errors were encountered: