Tests using MSBuildWorkspace are flaky #376
Comments
|
Pretty sure all the issues here come down to: |
|
You need to construct MSBuildWorkspace using vs-mef. Here's an example: Absent that, you must ensure that the tests using MSBuildWorkspace do not run in parallel. The default MEF container does not support parallel use even across two completely separate instances. |
|
Thanks 😄 I was thinking that, but definitely appreciate the confirmation since copying ~150 lines of code just to get a thread safe constructor was triggering alarm bells! (the referenced block, the export provider construction, and the contents of ExportProviderExtensions). |
GrahamTheCoder
added a commit
that referenced
this issue
May 8, 2020
beppe9000
added a commit
to beppe9000/CodeConverter
that referenced
this issue
Jul 27, 2020
* Changelog * Update README.md * Rough outline of dotnet tool that converts all vb in a solution to c# TODO: - MSBuild resolution (probably installing the right packages) - Args to deal with project files - Args to deal with overwriting - Args to deal with different output directory - Args to force skipping nuget restore - Args to force continuing with compile errors Spike of icsharpcode#555 * More args * Handle msbuild properties and projects * Rearrange * SDK version is missing, build against an older one * Allow use of another directory - fixes icsharpcode#217 * Copying fixups * Off topic: Use language names * Use any assemblies hanging around in the app domain too * Pull out project creation * Remove cache for comparison * Improve constructor generation * Parenthesize cast expressions * Run in dot net core and characterize reasonable changes * Use net core types in GlobalImportsStatement * Add missing using * Disable check The versions that come out in .net core are totally different * Try to preserve conversions to char and array for back-compat * Block all overwriting unless comitted to git or forced * Create DirectoryInfoExtensions * Use MSBuildWorkspaceConverter * Make static * Reorder * Don't dispose, let it be used by other tests since it's immutable * Attempts useing VS msbuild so system runtime types resolve * Fix git diff * Avoid yield because it's a linked file * Set AnyCPU to give auto-detection a chance * Try x64 * Run .net core tests * Spike targeting net48 Need to then figure out if we can use a command line argument to launch the net framework version based on a command line option If it works in this form, I can pull all the code into a netstandard library which the tests could reference (and even multi-target themselves if we wanted to test with both runtimes) * Patch in missing behaviour * Split out tons of parts to get around core vs framework nightmare * DotNetTool commands must be dot net core * Dot net core's msbuildlocator uses the net core sdk for workspace creation * The net core sdk can't resolve references for framework projects * Specifically locating msbuild for .net framework throws an error about a missing method - it's not compatible * Shared project * Review: Make some things public I don't really want people from other repos using these since they aren't stable APIs, just helpers. Might need to rethink how to share these * Revert test framework-related changes * Call one from the other * Tell users about this new switch when it inevitably causes pain Would be better off doing some simple detection on the project files probably. The framework version of the exe is actually more flexible since it works for framework and core, but it won't work on other platforms. It may make sense to just try to detect when the framework one is runnable. * Detect environment * Tell people about the keyboard shortcut for copying * Convert omitted delegate parameter types using symbol - fixes icsharpcode#560 * Make dot net core version the main named thing and fix up some warnings * Add a helper script for installing locally * Restore R# required dependencies * Revert nullable test changes * Show errors in tests * Revert readonly span hacks The code will only be run against .net core projects which will support them * Tweak caching to avoid bug in msbuild workspace * Install nuget.build.tasks to fix workspace level errors * Recharacterize wrong behaviour (same as start of branch) Only work in .net core at the moment * Await file writing task * Return same extension by default * Re-set target path after writing * Update readme * Wait for the output to be written * Set working directory in case it helps resolving dependencies * Use latest version over 16.0 - todo: warn if <16.0 is available * Load any assembly, and delete source files that aren't in target * Finalize copy and delegation behaviour * Avoid passing path to git because it's hard to know what format * Log loader exceptions * Give MSBuild locator first go at resolving depenencies + allow others * Don't copy binaries/caches * Check happy path * Tell users when we're ignoring their ancient software * Only limit git diff lines * Write exception message last so user sees it in console * Don't need a stack trace for validation errors that the user can fix * Avoid huge diffs and issues for non git users * Bump version to 8.1.0 * Remove appveyor.yml * Show help on debug install * Tidy up output when git is missing * Async test names to avoid warnings * Fixup most warnings/info items * Remove async suffix * Half package size Turns out they don't need to be content if they're marked as packed. When marked as content it gets included two extra times, and the zip format doesn't seem to compress that well * Move readme and add design notes * Add some future notes * Update README.md * Remove pretty unused code * dlls are binaries * Hack to include dataflow dll for 2017 * Stop binding assembly versions at all, and add version info * 8.1.1 * Update README.md * Update bug_report.md * Deal with array type symbols - fixes icsharpcode#559 * Fix test input compilation * Parenthesize all binary expression - fixes icsharpcode#565 Simplifier will deal with ones that weren't needed * Fix NuGet name * Converting ForEach loop: avoid duplicate variable compilation issue Fixes icsharpcode#558 * Neaten up case when variable is only used within loop * Use var except when casting * Improvements to for loop with missing semantic info - fixes icsharpcode#482 * Loop over enum * Split byref tests into own file * Ref improvements * Lots of WIP for generating a local function * Rename and add xml doc * Manually constructed test case * Widen the hoisting concept * Create local function (a bit eagerly and with a static name) * Less eagerly extract local function * Rename after * Convert if parts in expected order * Handle properties and missing semantic info * Handle other arg types * Handle type mismatches, constants, instance expressions * Release notes * Handle array element references * Pull out an extra temp for element access expressions * Add test for object list * Readability tweaks * Split out, simplify and neaten the output * Extract event tests into own class * Extract property member tests into own class * Extract operator member tests to own class * Use stack so nested types get correct initializers * Tidyups * Add as constructor initializer * Deal with initializers not in the part where constructor is created Fixes icsharpcode#281 * Split arrays with different ranks Part of icsharpcode#544 * Handle MidAssignmentStatement Part of icsharpcode#544 * Convert array type when there's an initializer Part of icsharpcode#544 * Version 8.1.2 * Textual improvements to vsix manifest * One way of solving test race condition * Use confirmed thread safe way of creating workspaces icsharpcode#376 (comment) * Pass the MSBuild default assemblies * Convert to async * Pass through joinable task factory * Rename -> Async * Use non-special hex values * Make switch case labels const where possible Fixes part of icsharpcode#544 * Change the name to avoid icsharpcode#569 * Extract logic * Assign names back to their originals - fixes icsharpcode#569 * Remove explicit cast where possible - fixes icsharpcode#388 * If no project file: Hint at installing the VisualBasic nuget package (for the relevant using statements) - Relates to icsharpcode#388 * Improve performance of VB->CS single file conversion - fixes icsharpcode#469 Avoid converting My namespace which is thrown away anyway * Characterize this oddity * fix typo in readme * Add AsEnumerable in linq "in" clause - fixes icsharpcode#544 * Tidy up string comparison - fixes icsharpcode#500 * Update changelog * Update changelog * Initial copy * Pull in what's needed to get this building in its current form * Move in existing mygroup functionality * Move existing exponentiation logic * Move existing like operator * Fix race condition in GetExitCodeAsync(). After `WaitForExitAsync`, the git process has written all its output to the pipes and terminated. However, it's possible that CodeConverter process has not yet read the data from the pipe, or that `OutputDataReceived` event is still executing after `GetExitCodeAsync()` returns. This can cause an `ObjectDisposedException` if the caller manages to dispose the `StringWriter` before the data is read from the pipe. The `OutputDataReceived` event is called once with `e.Data == null` to signify the end-of-output. Waiting for that prevents the race condition. * Introduce KnownMethod to save inlining 1000s of lines of hardcoding * Modulo should have conversion * Subtle behaviour change: Use bool version rather than late bind * Remove unused using (There's a global import) * Indexers most commonly have 1 arg * Use Array.Resize - fixes icsharpcode#501 * Use C# 7.3 compatible null check and simplify unary negation * Version 8.1.3 * Fix syntax tree issue * Test tweaks * Don't qualify non-static generic names * Don't do any extra qualification for generic names Doesn't cause any negative effects on tests * Fix more predefined type name parsing * Add ref keyword for Array.Resize * Use correct initializer type * Don't use private keyword when already declared private * Nameof expressions contain member access, rather than qualified names * Update comments * Update CHANGELOG.md * Flip condition - a type name should never be a method call * Fix typo * Create delegating method for implementations - fixes icsharpcode#443 - fixes icsharpcode#444 * Improve MyClass tests * Deal with more combinations of explicit / parameters / myclass * Write to temp path - hopefully fixes icsharpcode#578 * Add version * Avoid nullref * Update README.md * Update CodeConverter.csproj * Update description -fixes icsharpcode#577 * Update README.md * Fall back on compilation option * Dedupe and fix syntaxkind * Coerce to string in option compare text for char arrays * Formatting only * Omit conversion * Add test with option compare text true * Cater for nulls too * Pass through comparison and use invocation style * Wrap all cases for case insensitive strings * Deal with case insensitive comparisons * Try to minimize the number of null coalesces * Add self verifying tests * 8.1.4 * Deal with as many combinations of cases as possible * Changelog * Use var pattern to reduce scope of variable - fixes icsharpcode#323 * Only cast when switching on enum * Fix and simplify * Update CONTRIBUTING.md * Fix non-string case regression - fixes icsharpcode#585 * Improve numeric casts - fixes icsharpcode#580 * Set refkind for raise event invocations - fixes icsharpcode#584 * Refactor existing renamer * Rename clashing type members - fixes icsharpcode#420 * Fix string -> enum - fixes icsharpcode#476 * Make comment clearer * Rename explicit method implementations where needed - fixes icsharpcode#492 * Include type info in conversion of default(someType) - fixes icsharpcode#486 * Do a single field exchange, in case someone else exchanges handlers * Factor out class to minimize chance of mixing up two handlers * Report renaming difficulties * Use workspace options * Workaround VS caching issue - fixes icsharpcode#586 * Only set if not already set * Set to the Visual studio JoinableTaskFactory * If there's no symbol we'd need to write a whole different impl * Add to changelog * 8.1.5 * Set file path for text only conversion - fixes icsharpcode#589 * Correct logic for conversion objWithOverloads Is Nothing - fixes icsharpcode#591 * Use object pattern * Correctly use numeric value - fixes icsharpcode#590 * LangVersion 8 * Increase default language version * Switch to "is null" * Correct conversion for equality of overloaded types - fixes icsharpcode#594 * Try to mimimize flakiness due to non-determinism * Improve heuristic for unknown indexer vs method, fix missing params Relates to icsharpcode#595 * Attempted fix for icsharpcode#592 I suspect different visual studio versions are parsing nameof differently. Explicitylt constructing an identifier with the syntax kind may get around this. * 8.1.6 * Revert "Increase default language version" This reverts commit d66da69. * Optional parameters in parameterized properties - fixes icsharpcode#597 * Remove unused and fix typo * Ensure built before conversion Relates to icsharpcode#592 Co-authored-by: GrahamTheCoder <grahamthecoder@gmail.com> Co-authored-by: Christoph Wille <christoph.wille@gmail.com> Co-authored-by: Blubbll <34196146+blubbll@users.noreply.github.com> Co-authored-by: Daniel Grunwald <daniel@danielgrunwald.de>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The MSBuildWorkspace (only used in 4 tests) occasionally sees two different issues. A NullReferenceException, and (more rarely) this one:
We should see if we can upgrade to a later version that fixes the issue
I believe the nullref in optionsservice is some kind of multiple initialization issue (possible a multithreading issue)
Could be related to dotnet/roslyn#17401 though that doesn't mention the flakiness seen here
The text was updated successfully, but these errors were encountered: