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

Support VSTest parallel test execution setting #2385

Merged
merged 4 commits into from
Aug 26, 2019

Conversation

dungpa
Copy link
Contributor

@dungpa dungpa commented Aug 26, 2019

This PR enables the use of /Parallel setting in vstest.console.exe.

I haven't added /Parallel to the legacy counterpart at https://github.com/fsharp/FAKE/blob/1f8de0538209376f73e964f71957ddb6dabaa45c/src/legacy/FakeLib/UnitTest/VSTest.fs#L34. Please let me know if the setting is also required there.

See these references for more details about the setting:

TODO

Feel free to open the PR and ask for help

  • New (API-)documentation for new features exist (Note: API-docs are enough, additional docs are in help/markdown)

  • unit or integration test exists (or short reasoning why it doesn't make sense)

    Note: Consider using the CreateProcess API which can be tested more easily, see https://github.com/fsharp/FAKE/pull/2131/files#diff-4fb4a77e110fbbe8210205dfe022389b for an example (the changes in the DotNet.Testing.NUnit module)

  • boy scout rule: "leave the code behind in a better state than you found it" (fix warnings, obsolete members or code-style in the places you worked in)

  • Fake 5 API guideline is honored

@matthid
Copy link
Member

matthid commented Aug 26, 2019

Thanks for looking into this! I noticed this module doesn't use properly escaped APIs and has no tests. Maybe you could clean it up?

@dungpa
Copy link
Contributor Author

dungpa commented Aug 26, 2019

Could you give some pointers where we use properly escaped APIs and test samples that I can follow? (Sorry, I haven't looked at the code base for a long time).

@matthid
Copy link
Member

matthid commented Aug 26, 2019

Sure, we have https://fake.build/core-process.html and in particular the Arguments API

In the end it should look similar to the StringBuilder which is currently used, but we can have a internal "helper" function returning the Arguments object which we can write tests against. Just look at the existing tests.

@dungpa
Copy link
Contributor Author

dungpa commented Aug 26, 2019

@matthid I added tests as requested.

Copy link
Member

@matthid matthid left a comment

Choose a reason for hiding this comment

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

Ah I didn't realize this only writes an argument file. Anyway, thanks for adding the test!

@matthid matthid merged commit 452895d into fsprojects:release/next Aug 26, 2019
@dungpa dungpa deleted the vstest2 branch August 26, 2019 20:02
@matthid matthid mentioned this pull request Oct 12, 2019
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.

2 participants