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

NUnit 3 support #1064

Merged
merged 1 commit into from Jan 11, 2016
Merged

NUnit 3 support #1064

merged 1 commit into from Jan 11, 2016

Conversation

fstaffa
Copy link
Contributor

@fstaffa fstaffa commented Jan 5, 2016

  • support for NUnit3, based on NUnit 2 support
  • no compatibility layer for NUnit2, users will have to create the arguments from scratch
  • tested all supported parameters and they work fine
  • missing - automated tests, is there some way how such extension would be tested in Fake ?
  • missing - do I need to test somehow that support for TeamCity working directory works as expected? I took the logic from NUnit client and I am not sure what is the expected behavior

@fstaffa fstaffa mentioned this pull request Jan 5, 2016
ToolPath : string

/// NUnit console runner name. ( `nunit3-console.exe`)
ToolName : string
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need ToolPath & ToolName? Most (or all?) other helpers just have a ToolPath parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, will merge these two.

@fstaffa fstaffa changed the title NUnit 3 support (WIP) NUnit 3 support Jan 9, 2016
@forki
Copy link
Member

forki commented Jan 10, 2016

Is this ready to merge?

@fstaffa
Copy link
Contributor Author

fstaffa commented Jan 10, 2016

It is. I have tested it with all in this wrapper and it works fine.
@inosik proposed changing the argument of Framework from string to discriminated union but it probably won't bring much benefit.

@forki
Copy link
Member

forki commented Jan 10, 2016

ok then please change it now, so that we don't have breaking change later

@fstaffa
Copy link
Contributor Author

fstaffa commented Jan 10, 2016

Ok, will add there common runtimes which are supported by NUnit 3 and we can expand them later as needed

@fstaffa
Copy link
Contributor Author

fstaffa commented Jan 10, 2016

I have changed runtime to discriminated union. I have predefined .net frameworks (2.0, 3.5, 4.0, 4.5) and added Other for any other value.
I was not able to get it running (or get NUnit3 running at all) with mono runtime so I didn't predefine any mono runtime and it will have to be defined manually using Other.
Otherwise this is ready for merge.

@fstaffa
Copy link
Contributor Author

fstaffa commented Jan 10, 2016

Thank you, this looks good. I added them (except frameworks 1.0 and 1.1 which should not be supported by NUnit 3)

@johnkors
Copy link

+1 on this feature. :)

forki added a commit that referenced this pull request Jan 11, 2016
@forki forki merged commit 88d730f into fsprojects:master Jan 11, 2016
@forki
Copy link
Member

forki commented Jan 11, 2016

thanks a lot

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