Rewrite xunit tasks #800
Rewrite xunit tasks #800
Conversation
|
One remaining concern in my mind is whether or not we want the |
|
@ploeh could you please review if this goes into your desired direction? |
|
As a general observation, it seems to be going in the correct direction. However, I tried to pull down the fork and use it with a small code base of mine, and it doesn't work: As you can see, the offending command line option is However, looking at the generated command line expression, the rest looks correct. If I remove |
|
Yep, I misinterpreted how xUnit 2 interprets the |
|
Actually, I interpreted how xUnit 2 will interpret the |
|
I think I have an idea about how to reduce the pain of the breaking change. In general, if we are introducing a breaking change, then we are in for a penny; we are in for a pound. It's better to make the breaking changes wholesale and get to the API we want rather than have several piecemeal changes with breaking impedance at each step. My idea on how to resolve this is to keep the old tasks around, but mark them |
|
Yes creating copies and making the old one obsolete is good. |
neoeinstein
force-pushed
the
rewrite-xunit-tasks
branch
from
f77a49e
to
3f2b23c
Compare
This commit also introduces an xUnitSingle task which expects only a single assembly. Because xUnit v1 only accepts one assembly at a time, the xUnit task delegates to run each assembly in series. In doing that delegation, the xUnit task intercepts the report path parameters to inject the assembly name for each tested assembly. (Closes fsprojects#568)
This commit allows the parallel execution of multiple xUnit test assemblies from a single task using the parallelism features in the xUnit console runner. (Fixes fsprojects#797)
neoeinstein
force-pushed
the
rewrite-xunit-tasks
branch
from
3f2b23c
to
ca7d9ef
Compare
|
Ok. This should be ready for review. |
| |> appendTraits parameters.ExcludeTraits "-notrait" | ||
| |> toText | ||
|
|
||
| module internal ResultHandling = |
There was a problem hiding this comment.
I wonder if we should use Chessie.
|
@ploeh what do you say? Release as new minor rev? |
|
I just tried with my small 'test' project, and it looks great: It also works as expected if I turn off parallelisation: In addition, it also works in the old mode, and as expected, I get a compiler warning because the old mode is deprecated. So it looks good from my perspective 👍 |
|
ok then let's do this. |
|
released in 3.33. Thanks. 👍 |
|
Thank you both 🎉 😄 |
|
Just recently discovered this breaking change while we auto-updated FAKE. I kind of understand that FAKE as an application may not adhere to SemVer constraints. However, I would suggest to think about how to deal with breaking changes on helper/type level. |
|
@ilkerde, did it break your build script? I recently upgraded one of my projects to this version, and while my script gets a compiler warning about |
|
I just went through the (slight) pain of upgrading and was wondering, did anyone blog about these changes? I love the work and a simple blog post covering the breaking changes would be awesome. |
|
In particular the change to how test result paths are handled feels like a tiny loss, in that I quite liked the way this appendIfTrue parameters.XmlOutput (sprintf "-xml\" \"%s" (dir @@ (name + ".xml")))used the assembly name. |
|
Check the docs. The example shows how to get the previous naming convention even with the new method. |
|
😨 I really searched high and low and couldn't find any docs explaining this. Do you mean here? http://fsharp.github.io/FAKE/apidocs/fake-testing-xunit2.html Sorry if I'm just being dense here. |
|
Sorry, I'm responding from a dumb terminal, so I can't be as expressive as I'd like. The xUnit (v1) docs has a sample usage with the case you desire. (Same link, just drop the 2 in the url.) Basically, use |
|
But where do you get "name" from? Previously the path was generated for each assembly from its name, now you set one path for the config which is applied to all assemblies which means the file will be overwritten, no? |
This pull request supersedes #798 and proposes a fix for #797.
Changes in the
xUnitandxUnit2tasks are breaking. There is an addition of anxUnitSingletask which explicitly indicates that it takes a single assembly for xUnit testing to allow a consumer to have more control over how the target is consumed since xUnit v1 only accepts one assembly at a time.This PR also includes changes to allow specifying the reporting file names for HTML, XML, and NUnit-style reports as well as additional documentation and some refactoring to limit the exposed interface. Changes relevant to each task have been staged as separate commits.
Specifications have been added to the test project for all of these changes. Once merged, consumers who used the following xUnit parameters will break:
XUnitParamsConfigFile- Removed: Was not being used anywayHtmlOutput- Removed: Pass in report file name path asHtmlOutputPathXmlOutput- Removed: Pass in report file name path asXmlOutputPathNUnitXmlOutput- Removed: Pass in report file name path asNUnitXmlOutputPathWorkingDir- Type change: Now accepts astring optionVerbose- Removed: Pass in inverted value asSilentOutputDir- Removed: No longer usedIncludeTraits- Type change: Now accepts a list of name-value tuplesExcludeTraits- Type change: Now accepts a list of name-value tuplesXUnit2ParamsConfigFile- Removed: Was not being used anywayParallel- Type change: Now accepts a discriminated unionParallelOptionMaxThreads- Type change: Now accepts a discriminated unionMaxThreadsOptionHtmlOutput- Removed: Pass in report file name path asHtmlOutputPathXmlOutput- Removed: Pass in report file name path asXmlOutputPathXmlV1Output- Removed: Pass in report file name path asXmlV1OutputPathWorkingDir- Type change: Now accepts astring optionOutputDir- Removed: No longer usedIncludeTraits- Type change: Now accepts a list of name-value tuplesExcludeTraits- Type change: Now accepts a list of name-value tuplesTeamcity- Renamed:ForceTeamCityAppveyor- Renamed:ForceAppVeyor