Update framework, consolerunner, and engine #218
Conversation
* NUnit framework updated to 3.7.1
* NUnit.ConsoleRunner updated to 3.7.0
* NUnit.Engine updated to 3.7.0
Fixes #199
|
Failing on Travis. If you can get it fixed we can merge, but normally we don't want to arbitrarily update our dependencies. In addition, we wouldn't normally update all these dependencies at the same time or for the same reason. The engine is our only true dependency. The gui is written assuming certain features in the engine. In theory, the engine is always backwards compatible, but now that this project is separate from the NUnit Project, we need to actually test new engine releases before using them. The framewok is only a dependency of our tests. We can update the version as needed but it doesn't have anything to do with how the gui works. In fact, it's supposed to work with any framework version. The console runner is used to run our tests in batch mode. It doesn't affect the gui either although we have to watch out for engine conflicts, since the console runner comes with it's own copy of the engine. As I said, even though some of this is unnecessary, it can't hurt provided you can get the CI to pass. If not, we can remove whichever upgrade is causing the problem. |
|
Originally I just wanted a newer version of the engine, so we could get some better logging for #162, and then I thought that would update it all, as I had to touch the same files (except for the console runner). Also I did not think that it would break anything. As far as I can tell (by googling the error message from Travis) the problem is that the nuget-client used by the script is too old. The current cake script downloads nuget.exe from https://www.nuget.org/nuget.exe (which is version 2.8.60717.93), but newer cake scripts (like the one used for nunit-console) downloads from https://dist.nuget.org/win-x86-commandline/latest/nuget.exe (which is version 4.1.0.2450). Would it be okay if I also updated the cake bootstrappers (build.ps1 and build.sh) with the newest files? |
|
That makes sense to me. If necessary, you can update the Cake version as well. |
Downloaded new bootstrappers from https://github.com/cake-build/resources and removed tools/nuget.exe, so a newer version can be downloaded by the scripts.
|
Odd message in travis: "More than one build script specified" |
|
Yes I'm looking into it 😟 . I guess that this will take some tries, as I cannot test my changes locally. |
These defaults have been remove from build.sh
|
It is working again 😄 . When updating the bootstrappers to the newest version it required changes in .travis.yml to keep the existing behaviour. I think that the changes from @jnm2 in cake-build/resources#30 required us to change Ps. I've also set
I'll create two issues regarding these problems. PPs. @CharliePoole Is it on purpose that we do not run tests on Travis? |
|
Frankly, I don't like that change. It goes against standard conventions for use of Re: Warnings - I was aware of the second but not the first. We shouldn't have any .NET Core in our build. Re: Tests - I haven't been able to run them in Travis in the past but I'm working on something. |
If that changed, that was not on purpose and I need to get that fixed. |
|
I assumed that cake.exe interpreted |
|
That's the Cake folks' call, of course. They may want to use something consistent with Powershell in both environments. |
|
Okay right now, |
There was a problem hiding this comment.
@mikkelbu I was wondering what had happened to this PR but when I looked at it I saw that my comments are still all pending! I'm making this review a Comment for now so you can read the comments By now I have forgotten myself what they were! Once past that, we can decide what to do next.
| @@ -4,5 +4,5 @@ mono: | |||
| - latest | |||
| - 4.6.2 | |||
| script: | |||
| - ./build.sh --target "Travis" | |||
| - ./build.sh -target="Travis" -configuration="Release" -verbosity="verbose" | |||
There was a problem hiding this comment.
The use of -- options is normal for non-single-letter options in Linux. I'm surprised the single dash works at all, since --targets is supposed to mean the same as -t -a -r -g -e -t -s in normal use. Has the script changed WRT this?
There was a problem hiding this comment.
Apparently, this is due to Mono/Cake. Before the build.sh script did special-handling of, among other, target
https://github.com/NUnitSoftware/nunit-gui/blob/7fb774fc4b5e4e52107bb17a46e30442c38a0cc0/build.sh#L25
(e.g. extracted the value into TARGET and then called Mono/Cake in the end of the script like this
https://github.com/NUnitSoftware/nunit-gui/blob/7fb774fc4b5e4e52107bb17a46e30442c38a0cc0/build.sh#L61
In the version of the script I used, there was no special-handling of target. Hence I had to give the parameters in the syntax that Mono/Cake expects. I think that @jnm2 change above will solve this.
Fixes #199