Skip to content
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

Integration Test Infrastructure #1834

Merged
merged 5 commits into from
Nov 6, 2017

Conversation

TheRealPiotrP
Copy link

I'm splitting up #1826 into an infra PR and a feature PR. I'd like to merge the infra ASAP. Once this is done, I'll add some integration tests focused on the launch.json/tasks.json interactions. Specifically, I'd like some tests that are able to debug starting at a clean solution to validate the interactions between launch.json and tasks.json. Once this is in-place, I will add back the taskProvider work which will piggy-back on top of the aforementioned tests.

I would like not to squash this PR. I have created meaningful commits which separate the creation of the new infrastructure from the creation of test assets. Folks wishing to add new test assets can use the existing commits as a tempalte.

Test Infrastructure Changes

My strategy in updating the test infrastructure is two-fold. First, I wanted to be able to run integration tests in the context of representative workspaces. Second, I wanted to be able to access data shared with VS Code which is not subsequently queryable via VS Code API's. Here are the key components of the change:

Test Assets

I introduced a new top-level folder called TestAssets. Within this folder you will find three representative workspaces:

  • A workspace with a single csproj-based project
  • A workspace with a sln file and several related projects
  • A workspace with a single project.json-based project

These workspaces are accompanied by matching .ts files which describe, from a test expectations perspective, the contents of the workspaces.

omnisharp-vscode launch.json updates [does not affect the extension-produced launch.json]

The launch.json has been updated to have test execution configurations for each of these workspaces. Selecting a given configuration will open the VS Code Test Host targetted at the specified TestAsset workspace. All of the tests running within that Test Host will then be interacting directly with the target workspace, allowing them to produce predictable results while exhibiting their ability to reason about that wokspace type. Note that in order to run through all test scenarios you must iterate through all available Test Configurations.

package.json updates

package.json was updated to allow cmd-line execution of each of the new Test Configurations. The base npm test command was updated to iterate through all three configurations, thereby ensuring that a single cmd-line invocation will execute all tests.

@@ -7,6 +7,7 @@
"request": "launch",
"runtimeExecutable": "${execPath}",
"args": [
"${workspaceRoot}/test/integrationTests/testAssets/slnWithCsproj",
Copy link
Member

Choose a reason for hiding this comment

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

I'm still not a super fan of this particular change because it'll make it more of a pain to debug customer repros. I've often had a customer repro project and relied on the fact that subsequent F5 invocations (after the first one) would load the previously loaded workspace. This would require me to change launch.json to get that behavior.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I'll revert this to unblock your workflow.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! I think it looks good to merge otherwise.

@DustinCampbell
Copy link
Member

Out of curiosity, will npm test now fail on High Sierra because of project.json?

@TheRealPiotrP
Copy link
Author

I did not merge project.json in this PR. I have it in a private branch if we decide we need it.

That said, even if it failed, you can run the integration test projects individually to work around such platform-specific issues!

@DustinCampbell
Copy link
Member

That said, even if it failed, you can run the integration test projects individually to work around such platform-specific issues!

Don't forget that project.json is already deprecated by the extension. 😄

@TheRealPiotrP TheRealPiotrP merged commit 2e1ee60 into dotnet:master Nov 6, 2017
@TheRealPiotrP TheRealPiotrP deleted the dev/piotrp/testInfra branch November 6, 2017 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants