adamchester / expecto-adapter

Visual Studio test adapter for Expecto (https://github.com/haf/expecto)
MIT License
28 stars 16 forks source link

It seems like we get a "lock" on the test exe #17

Closed mastoj closed 7 years ago

mastoj commented 7 years ago

I can only run the tests one or two times, after that it seems like Visual Studio, or something, has a "lock" on the test exe so my build fails. I get a couple of:

1>C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets(3989,5): warning MSB3026: Could not copy "obj\Debug\FSharpIntro.Tests.exe" to "bin\Debug\FSharpIntro.Tests.exe". Beginning retry 1 in 1000ms. The process cannot access the file 'bin\Debug\FSharpIntro.Tests.exe' because it is being used by another process.
1>C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets(3989,5): warning MSB3026: Could not copy "obj\Debug\FSharpIntro.Tests.exe" to "bin\Debug\FSharpIntro.Tests.exe". Beginning retry 2 in 1000ms. The process cannot access the file 'bin\Debug\FSharpIntro.Tests.exe' because it is being used by another process.

Then it fails with:

1>C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets(3989,5): error MSB3027: Could not copy "obj\Debug\FSharpIntro.Tests.exe" to "bin\Debug\FSharpIntro.Tests.exe". Exceeded retry count of 10. Failed.
1>C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets(3989,5): error MSB3021: Unable to copy file "obj\Debug\FSharpIntro.Tests.exe" to "bin\Debug\FSharpIntro.Tests.exe". The process cannot access the file 'bin\Debug\FSharpIntro.Tests.exe' because it is being used by another process.

Is it a known issue?

adamchester commented 7 years ago

@mastoj thanks for reporting this. I have a wild guess that it might have something to do with Mono.Cecil, because that is one thing that changed a lot in recent versions.

Is this a new issue? what version(s) of things do you have?

mastoj commented 7 years ago

@adamchester I used the newest stable version of everything related to Expecto expect for the runner where I used the latest pre release. I had to fall back to FsUnit though since I'm preparing a workshop and need the tools to work for everyone at the workshop.

I'm using VS2017 if that makes a difference.

Zeroto commented 7 years ago

I had the same problem. Using expecto 4.1.1 with the 5.0.0-alpha008 testadapter it caused locks on the file. By reverting to 4.0.0-alpha007 it started working. This is in vs2017.

MNie commented 7 years ago

@Zeroto how do You use 5.0.0-alpha008 if this version works only with Expecto in version > 5.0? If you are using Expecto 4.1.1, you should use TestAdapter in version at most 4.0.0-alpha007 @mastoj Did you something after this 1/2 successful runs? Install/Update package or something?

mastoj commented 7 years ago

I might have updated everything I could update, but only stable versions. The only pre release package was this I think? Don't remember exactly what I did, sorry. On Sun, 23 Apr 2017 at 22:10, Michał Niegrzybowski notifications@github.com wrote:

@Zeroto https://github.com/Zeroto how do You use 5.0.0-alpha008 if this version works only with Expecto in version > 5.0 https://www.nuget.org/packages/Expecto.VisualStudio.TestAdapter/? If you are using Expecto 4.1.1, you should use TestAdapter in version at most 4.0.0-alpha007 https://www.nuget.org/packages/Expecto.VisualStudio.TestAdapter/4.0.0-alpha007 @mastoj https://github.com/mastoj Did you something after this 1/2 successful runs? Install/Update package or something?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/adamchester/expecto-adapter/issues/17#issuecomment-296485140, or mute the thread https://github.com/notifications/unsubscribe-auth/AAemsPuTssM4247z1tdkRH1zAbWCScRSks5ry7A1gaJpZM4NDk8g .

MNie commented 7 years ago

@mastoj I see. I ask you about this because I was trying to reproduce your problem and if I:

mastoj commented 7 years ago

I'm pretty sure I updated packages as you describe in point 2. On Mon, 24 Apr 2017 at 06:33, Michał Niegrzybowski notifications@github.com wrote:

@mastoj https://github.com/mastoj I see. I ask you about this because I was trying to reproduce your problem and if I:

  • Install Expecto ( > 5.0) and Adapter ( > 5.0) I could run test multiple times, change code and then also run specs -> tl;dr; everything works fine
  • Install Expecto ( > 5.0) and Adapter (> 5.0) run tests several times and then update/reinstall packages related to Expecto I get the same error as you get..

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/adamchester/expecto-adapter/issues/17#issuecomment-296522702, or mute the thread https://github.com/notifications/unsubscribe-auth/AAemsNxidmF2_ji3S-JsOVaF9aRl_Djvks5rzCYtgaJpZM4NDk8g .

Zeroto commented 7 years ago

@MNie That is not clear from the release description. As how I read it, it added support for expecto 5.0. Not removed support for expecto 4.0 and lower.

@mastoj It is good to note that expecto 5.0 is still a prerelease version. So if you installed the latest stable, then you are using expecto 4

kunjee17 commented 7 years ago

@mastoj hey, I was facing the same issue. Then I moved to Install-Package Expecto -Version 4.2.1 and it will also revert adaptor to previous version (4.*) everywhere. And things are working for me.

kunjee17 commented 7 years ago

@adamchester hey, I was unable to pin point the issue. As if I run the console project it will work. But if I try using Test Explorer it was unable to fine test projects as well as locking down the exe file in bin/debug. I couldn't even delete it manually without stopping the process from task manager. This things happens only when it was version is > 5.

marcpiechura commented 7 years ago

I have the same issue, according to the test discovery logs it tries to insert the same key twice into a dictionary.

------ Ermittlungstest gestartet ------
System.ArgumentException: Ein Element mit dem gleichen Schlüssel wurde bereits hinzugefügt.
   bei System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
   bei System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   bei Mono.Cecil.Cil.MethodDebugInformation.GetSequencePointMapping()
   bei SourceLocation.SourceLocationFinder.getFirstOrDefaultSequencePoint(MethodDefinition m)
   bei <StartupCode$Expecto-VisualStudio-TestAdapter>.$SourceLocation.candidateSequencePoints@71-2.Invoke(MethodDefinition m)
   bei Microsoft.FSharp.Collections.IEnumerator.map@107.DoMoveNext(b& )
   bei Microsoft.FSharp.Collections.IEnumerator.MapEnumerator`1.System-Collections-IEnumerator-MoveNext()
   bei <StartupCode$Expecto-VisualStudio-TestAdapter>.$SourceLocation.clo@41.GenerateNext(IEnumerable`1& next)
   bei Microsoft.FSharp.Core.CompilerServices.GeneratedSequenceBase`1.MoveNextImpl()
   bei Microsoft.FSharp.Core.CompilerServices.GeneratedSequenceBase`1.System-Collections-IEnumerator-MoveNext()
   bei Microsoft.FSharp.Collections.IEnumerator.map@107.DoMoveNext(b& )
   bei Microsoft.FSharp.Collections.IEnumerator.MapEnumerator`1.System-Collections-IEnumerator-MoveNext()
   bei Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.takeOuter@665[T,TResult](ConcatEnumerator`2 x, Unit unitVar0)
   bei System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   bei System.Linq.OrderedEnumerable`1.<GetEnumerator>d__1.MoveNext()
   bei Microsoft.FSharp.Collections.IEnumerator.map@107.DoMoveNext(b& )
   bei Microsoft.FSharp.Collections.IEnumerator.MapEnumerator`1.System-Collections-IEnumerator-MoveNext()
   bei System.Linq.Enumerable.<TakeIterator>d__24`1.MoveNext()
   bei Microsoft.FSharp.Collections.SeqModule.TryFind[T](FSharpFunc`2 predicate, IEnumerable`1 source)
   bei Discovery.Discoverer.Microsoft-VisualStudio-TestPlatform-ObjectModel-Adapter-ITestDiscoverer-DiscoverTests(IEnumerable`1 sources, IDiscoveryContext discoveryContext, IMessageLogger logger, ITestCaseDiscoverySink discoverySink)
========== Ermittlungstest abgeschlossen: 0 gefunden (0:00:00,7144686) ==========

Ein Element mit dem gleichen Schlüssel wurde bereits hinzugefügt. translates to A element with the same key was already added not sure if it helps.

jackfoxy commented 7 years ago

I am experiencing probably the same issue on VS2017 and VS2015, with the very latest test adapter and latest Expecto.

Expecto (5.0)
  Argu (>= 3.7)
  FSharp.Core (>= 4.1.12)
  Mono.Cecil (>= 0.10.0-beta5)
Expecto.BenchmarkDotNet (5.0)
  BenchmarkDotNet (>= 0.10.5)
  Expecto (>= 5.0)
  FSharp.Core (>= 4.1.12)
Expecto.FsCheck (5.0)
  Expecto (>= 5.0)
  FsCheck (>= 2.8)
  FSharp.Core (>= 4.1.12)
Expecto.VisualStudio.TestAdapter (5.0.0) - version_in_path: true
  Expecto (>= 5.0 < 6.0)

If I run just one test from the test window, then the next time I try to do a build there is a lock on the test project's exe.

C:\Program Files (x86)\MSBuild\14.0\bin\Microsoft.Common.CurrentVersion.targets(3813,5): warning MSB3026: Could not copy "obj\Debug\PersonalServer.Tests.exe" to "bin\Debug\PersonalServer.Tests.exe". Beginning retry 1 in 1000ms. The process cannot access the file 'bin\Debug\PersonalServer.Tests.exe' because it is being used by another process.
MNie commented 7 years ago

@jackfoxy issue was reported to a Mono.Cecil. I also found one interesting thing that if a test project is set to a console application instead of a library everything works fine (if I run tests from a visual studio test explorer).

jackfoxy commented 7 years ago

@MNie My test project in question is a console app. Perhaps my problem requires opening a new issue?

MNie commented 7 years ago

@jackfoxy I don't think so, it seems that it is exactly the same problem. Also I have no idea why it is working on my machine if I have project type set to console app :(

jackfoxy commented 7 years ago

@MNie and your paket lock file entries look the same? What version (and which updates) of visual studio? How about FSharp.Core? My Paket lock says

FSharp.Core (4.2.1)
jackfoxy commented 7 years ago

Also my .NET Target Framework for the Test project is 4.7

MNie commented 7 years ago

@jackfoxy I use nuget, here is my packages.config file:

<packages>
  <package id="Argu" version="3.7.0" targetFramework="net461" />
  <package id="Expecto" version="5.0.0" targetFramework="net461" />
  <package id="Expecto.VisualStudio.TestAdapter" version="5.0.0" targetFramework="net461" developmentDependency="true" />
  <package id="FSharp.Core" version="4.1.17" targetFramework="net461" />
  <package id="Mono.Cecil" version="0.10.0-beta5" targetFramework="net461" />
  <package id="System.ValueTuple" version="4.3.0" targetFramework="net461" />
</packages>

and project is set to .net framework 4.6.1.

jackfoxy commented 7 years ago

I tried downgrading to .net framework 4.6.1 and FSharp.Core 4.1.17, but same problem. I only tried this on VS2017.

Could it have something to do with Mono.Cecil being involved in your test project @MNie ? My project does not have that.

jbevain commented 7 years ago

There are 2 different issues here.

First the lock file. As noted in the change log on http://cecil.pe, Cecil 0.10 now keeps a handle on the underlying file by default. ModuleDefinition now implement IDisposable and the user will be responsible for disposing of a module.

The module that you open there:

https://github.com/adamchester/expecto-adapter/blob/1e67dba168506f67746ee8c34a614b0b87d0d09c/src/Expecto.VisualStudio.TestAdapter/SourceLocation.fs#L22

Is never disposed, triggering a potential lock.

You have 2 solutions, you can either:

a) Dispose of the module when done with it. b) Pass InMemory = true to the ReaderParameters. That will consume more memory, but you won't have to dispose the module, if its life-cycle is cumbersome to manage.

Now, there's a Cecil exception where when constructing a map of sequence points you get an exception. I've never seen that, and it might be the case that in some cases, the F# compiler emits weird debug symbols.

I haven't been able to reproduce the issue with the binaries provided by @MNie. I'd love to look at a dll + pdb that reproduces it.

MNie commented 7 years ago

@jbevain I will fix that with one of the solution that You proposed and then check if problem still occurs. Many thanks for taking a look on that!

Edit: I use use instead of let and locally it seems that everything works fine.

MNie commented 7 years ago

@jackfoxy @mastoj It seems that the newest version fix the problem with lock on an exe file.

jackfoxy commented 7 years ago

Great news. Hope to test it out tonight. I hope someone can find a solution for https://github.com/adamchester/expecto-adapter/issues/18

jackfoxy commented 7 years ago

Unfortunately for me this is actually worse. The Test Window fails in both VS2015 and VS2017


------ Load Playlist started ------
========== Load Playlist finished (0:00:00.0045009) ==========
------ Discover test started ------
System.ObjectDisposedException: Cannot access a closed file.
   at System.IO.__Error.FileNotOpen()
   at System.IO.FileStream.get_Position()
   at Mono.Cecil.Cil.CodeReader.MoveTo(MethodDefinition method)
   at Mono.Cecil.Cil.CodeReader.ReadMethodBody(MethodDefinition method)
   at Mono.Cecil.MethodDefinition.<>c.<get_Body>b__41_0(MethodDefinition method, MetadataReader reader)
   at Mono.Cecil.ModuleDefinition.Read[TItem,TRet](TRet& variable, TItem item, Func`3 read)
   at Mono.Cecil.MethodDefinition.get_Body()
   at Mono.Cecil.MethodDefinition.get_DebugInformation()
   at SourceLocation.SourceLocationFinder.getFirstOrDefaultSequencePoint(MethodDefinition m)
   at <StartupCode$Expecto-VisualStudio-TestAdapter>.$SourceLocation.candidateSequencePoints@71-2.Invoke(MethodDefinition m)
   at Microsoft.FSharp.Collections.IEnumerator.map@114.DoMoveNext(b& )
   at Microsoft.FSharp.Collections.IEnumerator.MapEnumerator`1.System-Collections-IEnumerator-MoveNext()
   at <StartupCode$Expecto-VisualStudio-TestAdapter>.$SourceLocation.clo@41.GenerateNext(IEnumerable`1& next)
   at Microsoft.FSharp.Core.CompilerServices.GeneratedSequenceBase`1.MoveNextImpl()
   at Microsoft.FSharp.Core.CompilerServices.GeneratedSequenceBase`1.System-Collections-IEnumerator-MoveNext()
   at Microsoft.FSharp.Collections.IEnumerator.map@114.DoMoveNext(b& )
   at Microsoft.FSharp.Collections.IEnumerator.MapEnumerator`1.System-Collections-IEnumerator-MoveNext()
   at Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.takeOuter@683[T,TResult](ConcatEnumerator`2 x, Unit unitVar0)
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.OrderedEnumerable`1.<GetEnumerator>d__1.MoveNext()
   at Microsoft.FSharp.Collections.IEnumerator.map@114.DoMoveNext(b& )
   at Microsoft.FSharp.Collections.IEnumerator.MapEnumerator`1.System-Collections-IEnumerator-MoveNext()
   at System.Linq.Enumerable.<TakeIterator>d__24`1.MoveNext()
   at Microsoft.FSharp.Collections.SeqModule.TryFind[T](FSharpFunc`2 predicate, IEnumerable`1 source)
   at Discovery.Discoverer.Microsoft-VisualStudio-TestPlatform-ObjectModel-Adapter-ITestDiscoverer-DiscoverTests(IEnumerable`1 sources, IDiscoveryContext discoveryContext, IMessageLogger logger, ITestCaseDiscoverySink discoverySink)
========== Discover test finished: 0 found (0:00:00.8720005) =========
jackfoxy commented 7 years ago

As an aside, I notice Expecto.VisualStudio.TestAdapter.dll does not have the proper assembly version stamped. Instead both the previous and new versions show file and product versions of 9999.0.0

MNie commented 7 years ago

@jackfoxy the steps to reproduce error that You get, are the same? (install package, run, uninstall, install, run). And maybe You could provide an exe file which fails for You so @jbevain could look at this, since exe file provided by me was 'correct'?

jackfoxy commented 7 years ago

I will try to get this together tonight.

jbevain commented 7 years ago

The exception @jackfoxy posted means that a Cecil method body was accessed after the module in which the method is defined has been disposed. Meaning the module is disposed too early.

MNie commented 7 years ago

@jbevain so the easiest solution to fix that problem right now, would be to use Pass InMemory = true to the ReaderParameters. instead of use/using block as it is done right now?

jbevain commented 7 years ago

That's indeed the easiest solution.

jackfoxy commented 7 years ago

@MNie test solution https://github.com/jackfoxy/personalServer use the issue17 branch. Steps: Open the solution in VS 2017 or 2015 and open the Test Explorer. You will see the bad results in the output window.

jackfoxy commented 7 years ago

Forgot to mention, but hopefully obvious, you need to do a Paket install on my test branch first.

jackfoxy commented 7 years ago

I noticed a commit, but no new release to Nuget. Is there something ready to test?

MNie commented 7 years ago

@jackfoxy Yes there was a commit with a second type of solution to a problem with not disposable ReadModule. But new package was not pushed to a nuget since it works for me the same as previous solution (I cannot reproduce lock on exe file right now). I think we could push new nuget version, so You could check if it helps You.

jackfoxy commented 7 years ago

@MNie did you try replicating on the test solution branch I posted above? Perhaps it's a setting I have in VS rather than the solution or VSIX.

adamchester commented 7 years ago

I'm not sure this has any impact on the problem we are discussing, but it's well known that visual studio test runner extensions can be corrupted / cached.

For example, the xUnit doco says:

If you're having problems discovering or running tests, you may be a victim of a corrupted runner cache inside Visual Studio. To clear this cache, shut down all instances of Visual Studio, then delete the folder %TEMP%\VisualStudioTestExplorerExtensions. Also make sure your project is only linked against a single version of the Visual Studio runner NuGet package (xunit.runner.visualstudio).

MNie commented 7 years ago

@jackfoxy not yet :(

jackfoxy commented 7 years ago

I just tried deleting VisualStudioTestExplorerExtensions folder and no change.

piaste commented 7 years ago

I encountered the same error as @jackfoxy on VS2017, using the 5.0.0.1 version.

I cloned the current master branch, packaged it, and referenced it (using paket.local). The problem went away.

So I just wanted to report that @MNie 's latest fix appears to work 👍

MNie commented 7 years ago

@jackfoxy @piaste 5.0.0.2 is available on nuget :)

jackfoxy commented 7 years ago

@MNie First the good news. With 5.0.0.2 I can see tests in the VS 2017 test explorer, run individual test, debug individual tests (setting breakpoints), and rebuild my solution, in about any arbitrary order I try, without locking files or any other problems.

Now the bad news. The test explorer is only discovering 48 of 153 tests. Also in VS2015. I tried cleaning the solution. I tried deleting the VS cache. Close reopen VS, etc. Same result.

The FAKE Expecto extension in by build script has no problem discovering and running all the tests.

I'll spend some more time to see if I can discover more information about the undiscovered tests.

Do we want to declare this issue resolved and open a new issue?

MNie commented 7 years ago

@jackfoxy

Excellent to hear that its finally works, but not so good that tests are not discovered :( I think You could open a new issue and If You could provide more info (visual studio version etc.) I would be very grateful!

jackfoxy commented 7 years ago

I'm pretty sure some test component (either this one or something in VS) is caching known tests at the test list level by module somewhere, I just don't know where. And it does not recognize that I have added more tests in this module. I tried commenting the tests that were showing up in this module, and they continued to be in the Test Explorer list!

I just need to figure out where this cache is. It is not the ComponentModelCache.

jackfoxy commented 7 years ago

Of course cleaning the solution from VS did not really clean it. And I determined there is no magic caching going on. I've isolated the problem area, and it has nothing to do with this issue.

So please mark this complete!

If I determine the new thing is a bug I will file a new issue.

jackfoxy commented 7 years ago

From my perspective this issue can be closed.

Is it still not working in some cases? @MNie

MNie commented 7 years ago

@jackfoxy I didn't know any cases when it is not working right now. If it occurs one more time we could reopen this issue :)