Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Conversation

@rmshaffer
Copy link
Contributor

@rmshaffer rmshaffer commented Aug 22, 2020

This PR re-enables temporary .csproj creation (essentially undoing #585) and also ensures that language server tests pass deterministically when opening both .qs files and non-.qs files.

There a few pieces to this fix, in addition to re-adding the code that was removed in #585:

  1. Only create temporary projects for .qs files. Some BasicFunctionality tests load randomly-named source files without a .qs extension, and we want to allow those tests to operate as they do today.

  2. Ensure that MSBuildLocator is registered for both the BasicFunctionality and ProjectLoader test classes via AssemblyInitialize attribute, rather than just a static constructor. With the static constructor approach, the order of execution was not deterministic, and if the BasicFunctionality tests were run prior to the ProjectLoader test class initialization, the proper version of MSBuild was not always being discovered. This actually led the tests to always pass, because the failure to create the temporary project was simply logged and the tests continued down their old path without a temporary project.

  3. Change the temporary project folder naming scheme so that it is stable. The previous GetHashCode() approach was not even stable within a single test run, which occasionally caused errors. This is a good change from a user perspective as well, since it will ensure that the same temporary project is always used for code in a given folder.

@rmshaffer rmshaffer requested a review from bettinaheim August 22, 2020 15:27
Copy link
Member

@msoeken msoeken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but I am not the expert for this one.

@rmshaffer rmshaffer marked this pull request as draft August 22, 2020 19:18
@rmshaffer rmshaffer marked this pull request as ready for review August 22, 2020 22:05
rmshaffer and others added 2 commits August 24, 2020 09:58
@rmshaffer rmshaffer changed the title Fix non-deterministic behavior in language server tests Enable temporary .csproj creation and associated language server tests Aug 24, 2020
@rmshaffer rmshaffer requested a review from bettinaheim August 28, 2020 21:25
@rmshaffer
Copy link
Contributor Author

rmshaffer commented Aug 28, 2020

@ricardo-espinoza @bettinaheim the tests are now passing reliably with the current set of changes, so I think it is ready to go, pending your review. Thanks!

@rmshaffer rmshaffer merged commit 8cb1d00 into master Sep 2, 2020
@rmshaffer rmshaffer deleted the rmshaffer/fix-language-server-tests branch September 2, 2020 15:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants