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

Added support for parallel processes & user specified arguments for xunit.console.exe #18

Closed
wants to merge 13 commits into from

Conversation

Jogge
Copy link
Contributor

@Jogge Jogge commented Oct 27, 2016

New features

  • Support for parallel processing of test assemblies.
    • Number of parallel processes can be configured in the settings of the plugin.
  • Support for user specified command line arguments for xunit.console.exe
  • Support for xUnit-2.2.0-beta3 (merge from https://github.com/NsdWorkBook/xUnit-TeamCity)

Nick Stenroos-Dam and others added 11 commits October 10, 2016 10:50
… existing handling prevented users from ignoring failed unit tests
…cess.java

	- Concurrent execution of xUnit test assemblies
 Conflicts:
	xunit-agent/src/main/java/se/capeit/dev/xunittestrunner/XUnitBuildProcess.java
…cess.java (BuildFinishedStatus):

  - Fixed compilation error introduced in merge conflict.  The log context is now kept in a dictionary.
@carlpett
Copy link
Owner

Hi,
Thanks for the PR! Especially the command line options part has been on the wish list for some time, it will enhance the plugin quite a bit!

Some points from reading the diff:

  • I just closed the PR you merged with, because I don't want to ship a beta version of xUnit. Could you remove those commits? I'll gladly accept a PR for including the new version when it is released :)
  • I don't understand what the regex does in building the command line? Also, it would be preferable to compile it outside the for-loop, to avoid paying the compilation cost for every assembly.
  • The help text for the command line options are only valid for 2.x versions of xUnit, and probably varies a bit between versions there as well? Ideally I think we should present the list for the selected version.

@robert-j-engdahl
Copy link

Regarding the regular expression: That is for generating the argv for "int main(int argc, char **argv). In particular it is not enough to split on the space character, as some options may be enclosed in quotes. So the regex basically just parses strings and quoted strings. I acknowledge that the .+?" thing is a bit unconventional.

@robert-j-engdahl
Copy link

The help text for command line options could perhaps be generated on the fly, if we can execute the xunit.console.exes from the plugin (we should just shown the output of running the command with no options)

@robert-j-engdahl
Copy link

Regarding the xunit beta version, I think that can be solved but just calling this version of the team city plugin a beta.

@carlpett
Copy link
Owner

@robert-j-engdahl Yes, but some comment string about the regex would be good, I think.

Regarding the help text, I can see several reasons to "hard code it":

  • The ui is generated on the TeamCity server, which may not be able to execute xunit.console for several reasons. It can be a Linux host, it may not have .NET installed, or it may not be a build agent (which would be recommended practice to not run both agent and server on the same host as you scale out), in which case the executable is not even deployed on the machine.
  • Forking and running the executable every time the build step page is shown seems likely to be very slow
  • We know exactly what text is displayed, and can make sure it fits/filter some uneccessary stuff out, etc.

@robert-j-engdahl
Copy link

You are right, it should just be inlined.

@carlpett
Copy link
Owner

@robert-j-engdahl @JoggeDK A bit confused regarding the authorship here, but could you please update the PR with the above?

mhoyer and others added 2 commits December 7, 2016 18:15
Fix missing safety check when parsing NumberOfParallelProcesses parameter
@carlpett
Copy link
Owner

Replaced by #20

@carlpett carlpett closed this Mar 31, 2017
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.

None yet

4 participants