Closed jonorossi closed 7 years ago
@fir3pho3nixx how do you think we should set this up?
Moving over the new tooling is a good idea. It gives you NuGet and AssemblyInfo built into the MsBuild which is really handy. You really don't want to break the old infrastructure whilst moving stuff across. For this I would recommend having two solutions.
'Castle.Core.sln'
Old TeamCity implementation.
'Castle.Core-VS2017.sln' (edited! from 'Castle.Core.Next.sln')
Shiny new AppVeyor(Windows) + TravisCI(MacOS, Ubuntu 14 LTS) version with GH status checks.
PR0: Move all the stuff in buildscripts into a deprecated folder but make sure that castle core build still passes on teamcity. This will make way for the new stuff.
PR1: Get the VS2017 project system building on AppVeyor using new tooling. Build only with DesktopCLR tests. Test parallelisation will start highlighting I/O collisions with ModuleScope for net35, net40, net45.
PR2: Get mono builds going on AppVeyor. This is important for packing NuGets but we might want to think about running this on Travis too. Up for grabs really.
PR3: Get tests passing for dotnet core on AppVeyor. We have the option of upgrading netstandard here but it currently introduces the conundrum of moving the tests over to xunit. Might need to help the nunit guys out if we dont wan't to migrate(Or not upgrade).
PR4: Get nuget publishing going from master branch only (merge to master = deploy). With GH integration for publishing releases(both draft and full). Might want to manually de-list the output for this, as it is not really a release of any kind.
PR5: Get tests passing for dotnet core(and potentially Mono) on TravisCI. This will yield some fun with getting libunwind to install on Ubuntu (Trusty Tahr). Luckily a solved problem for me.
PR6: Clean up and cut over.
Do you think we should consider adjusting this intended PR list to include the embedding of a proper build task framework(CAKE, FAKE, RAKE)?
You really don't want to break the old infrastructure whilst moving stuff across. For this I would recommend having two solutions.
I thought we'd just create a branch and start by deleting the old project files, and leaving master with TeamCity until we are ready to merge everything?
Test parallelisation will start highlighting I/O collisions with ModuleScope for net35, net40, net45.
I haven't had a chance to read in detail what the problem is here, but happy to work with you to sort this one out when the problem shows up in the build.
Get mono builds going on AppVeyor. This is important for packing NuGets but we might want to think about running this on Travis too.
We've only supported Mono on Linux+macOS not on Windows in the past, there was one point the Mono guys weren't even shipping a Windows build anymore. There really is little point for us to even look at Mono on Windows, no one uses it.
We have the option of upgrading netstandard here but it currently introduces the conundrum of moving the tests over to xunit.
Not sure what you mean here, but I think we should stay supporting .NET Standard 1.3 for now to support the widest of targets. Why would we need to move to xUnit.net for 1.6?
PR4: Get nuget publishing going from master branch only (merge to master = deploy). With GH integration for publishing releases(both draft and full). Might want to manually de-list the output for this, as it is not really a release of any kind.
I thought we'd drive NuGet.org publishing from tags. i.e. you tag "v1.2.3" in Git then AppVeyor builds and publishes "1.2.3" to NuGet.org and GitHub Releases tab. I'd prefer not to pollute NuGet.org with every single build from master, unlisted or not.
Do you think we should consider adjusting this intended PR list to include the embedding of a proper build task framework
We can look at it if it makes things heaps easier, but it seems the new .NET tooling takes away most of the craziness we've had to do in the past, some things we currently do in the build scripts isn't needed anymore. Would like to see if we can do it with a small shell script and the .NET CLI tooling.
Think the latest dotnet cli has trouble discovering tests for NUnit using their adapter. Moving over to xunit got tests to run easily but admittedly was the long way round. This might not be a problem, lets see how we go.
As for NuGet publishing and GH Tagging, AppVeyor can drive both. For changing the workflow to use tags as opposed to commits would probably need some more cfg listed here(but totally doable): https://www.appveyor.com/docs/branches/#build-on-tags-github-gitlab-and-bitbucket-only.
Get a branch going so I can start deleting the old build stuff and implement the new projects, whilst we decide how the appveyor build, test and publish workflow should work.
I've created the branch refactor-build
.
Do you have a link to the NUnit problem in their issue tracker?
Stumbled upon: https://github.com/nunit/nunit-console/issues/10 Potentially tracked here: https://github.com/nunit/nunit3-vs-adapter/issues/297
There are a few issues, with embedded resources. Will fix this next.
Ok, only 6(edited!) tests failing for net45. Almost ready to setup the netcoreapp1.1 tests.
Here is my simulated PR: https://github.com/fir3pho3nixx/Core/pull/3
Notice how status checks kicked in for me once I merged all the appveyor stuff into my default branch(master)?
Also noticed appveyor was having issues cloning from GitHub:
It went from this: https://ci.appveyor.com/project/fir3pho3nixx/core/build/4.0.4
To this(with a whitespace commit): https://ci.appveyor.com/project/fir3pho3nixx/core/build/4.0.6
Very close to achieving PR1.
@fir3pho3nixx Awesome work! Glad to see the rapid progress!
@jonorossi and @fir3pho3nixx please let us (my teammate @alinapopa and me) know if we can help on anything.
Made all the tests pass on appveyor for net45.
Here is my simulated PR:
https://github.com/fir3pho3nixx/Core/pull/3 https://ci.appveyor.com/project/fir3pho3nixx/core/build/4.0.19
Actual PR:
https://github.com/castleproject/Core/pull/244
@jeremymeng, @alinapopa would you be willing to start raising PR's on Castle Windsor using the same build technique I am using here in Castle Core? I will jump down from here once I have the tests passing on TravisCI and NuGets publishing using GH Tags.
This would set me up nicely for the migration to dotnet core properly on Castle.Windsor.
@jeremymeng, @alinapopa would you be willing to start raising PR's on Castle Windsor using the same build technique I am using here in Castle Core?
We are happy to. We should be able to start early next week.
I've stuffed around for ages trying to get AppVeyor to work with this repo, however GitHub just won't fire any webhooks (after the initial creation one). Webhooks work great on my personal fork, but also don't fire on https://github.com/castleproject/core-fork-for-webhook-test. I've contacted GitHub support to try to get to the bottom of it.
Just got a response from GitHub, below is the important snippet:
If an organization is disabled, GitHub will not deliver any webhooks. I see that your organization's billing is locked, which disables the organization. I've just cleared the flag on our side. Could you run another test event and let us know if you're hitting any issues?
Many years ago hammett had a paid plan for this GitHub organization which should have been downgraded, I don't remember why but after GItHub changed this flag webhooks are working now, they fired on that fork repo I created and are working for a tiny commit a just made to this repo.
Confirmed. Status checks are working: https://github.com/castleproject/Core/pull/247
Edited for TL;DR
To close out PR1, I created a branch that enables build output for all desktop clr framework monikers(net45, net451, net452 etc) and ran the tests in a single nunit console command quite a few times. Every now and then I would hit I/O collisions around 'CastleDynProxy2.dll'. The build mostly passed in my PR for appveyor but locally I was getting these failures every now and then:
1) TearDown Failed : Castle.DynamicProxy.Tests.InterceptorSelectorTestCase One or more child tests had errors
2) Error : Castle.DynamicProxy.Tests.InterceptorSelectorTestCase.Can_proxy_generic_interface TearDown : System.IO.IOException : Access is denied. (Exception from HRESULT: 0x80070005 (E_ACCESSDENIED)) --TearDown at System.Reflection.Emit.ModuleBuilder.SavePEFile(RuntimeModule module, String fileName, Int32 entryPoint, Int32 isExe, Boolean isManifestFile) at System.Reflection.Emit.ModuleBuilder.Save(String fileName, Boolean isAssemblyFile, PortableExecutableKinds portableExecutableKind, ImageFileMachine imageFileMachine) at System.Reflection.Emit.AssemblyBuilder.SaveNoLock(String assemblyFileName, PortableExecutableKinds portableExecutableKind, ImageFileMachine imageFileMachine) at System.Reflection.Emit.AssemblyBuilder.Save(String assemblyFileName, PortableExecutableKinds portableExecutableKind, ImageFileMachine imageFileMachine) at Castle.DynamicProxy.ModuleScope.SaveAssembly(Boolean strongNamed) at Castle.DynamicProxy.Tests.BasePEVerifyTestCase.TearDown()
3) Error : Castle.DynamicProxy.Tests.InterceptorSelectorTestCase.Can_proxy_same_type_with_and_without_selector_ClassProxy TearDown : System.UnauthorizedAccessException : Access to the path 'C:\code\Core\src\Castle.Core.Tests\bin\Release\net46\CastleDynProxy2.dll' is denied. --TearDown at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath) at System.IO.File.InternalDelete(String path, Boolean checkHost) at Castle.DynamicProxy.ModuleScope.SaveAssembly(Boolean strongNamed) at Castle.DynamicProxy.Tests.BasePEVerifyTestCase.TearDown()
I know this is a very contrived example but do you think this is anything to be worried about @jonorossi? Will skip working on PR2 and start working on PR3 next.
I noticed that the output seems to be running the different targets in different directories, it doesn't appear the be the problem I thought you are describing:
What I find interesting is that even though the framework folders are different and it passes in isolation for each framework on it's own, it should be able to scale up for multiple frameworks using multiple folders. Why would net461 have I/O collisions in it' own folder? Weird right?
What I find interesting is that even though the framework folders are different and it passes in isolation for each framework on it's own, it should be able to scale up for multiple frameworks using multiple folders. Why would net461 have I/O collisions in it' own folder? Weird right?
It looks like the NUnit guys didn't turn on parallel test execution within the same assembly by default in NUnit 3, so it definitely is strange. Have you tried running nunit-console with --agents=1
to confirm the problem is caused by running multiple assemblies at the same time?
Ran this a fair few times throughout the day on another machine, could not replicate it. Tried it again on same machine that kicked the errors out, all passed. I think what we need here is a brute force long distance test run, something that kicks out a mini dump(or similar), failing test name and some parent process info for when Castle.DynamicProxy.ModuleScope.SaveAssembly(Boolean strongNamed) tries to delete 'CastleDynProxy2.dll' in a test run and fails because of a file lock. Should we make this a separate issue and I keep chasing the new build stuff?
Should we make this a separate issue and I keep chasing the new build stuff?
Yes, a new issue definitely. It shouldn't hold up the build refactor work.
Rather than submit a crazy PR which does not make sense, I opt for discussion.
I am at a point where I can make Castle.Core tests build on dotnet core(targetting netcoreapp1.1 and net462 <- this is arbitrary btw). The good news is that it is entirely possible. The bad news is that all of the build symbols(flags) that are applied in the 'production code' (Castle.Core.dll) have to be applied to the tests(Castle.Core.Tests.dll). FEATURE_DICTIONARYADAPTER_XML
being a case in point because of incompatiblity with the System.Xml.Xsl stuff (xml etc ad infinitum).
Can we start with a series of PR's that address this? This does mean we have to change code in tests to make this work.
@fir3pho3nixx what issue are you seeing? Previously the test project only defines one additional symbol.
Thanks @jeremymeng, that does help avoid things like this: https://github.com/fir3pho3nixx/Core/commit/6942119ad991b43e16ea1284bf603e4780c9dadd#diff-9a410267b70c7f94379f9901fae787afR55
Problems are mainly around getting anything using the System.Xml.Xsl namespace to compile. Specifically things like CastleTests.Components.DictionaryAdapter.Xml.Tests.MockXPathFunction. Either I am missing a NuGet (which I hope is the case) or tests here need to start honouring the FEATURE_DICTIONARYADAPTER_XML
build symbol.
NuGets I am pulling in are here: https://github.com/fir3pho3nixx/Core/commit/6942119ad991b43e16ea1284bf603e4780c9dadd#diff-858b4452841160ed55e2d0a3a92ea51fR32
I ended up doing something really ugly to make it work which was(no future in this): https://github.com/fir3pho3nixx/Core/commit/6942119ad991b43e16ea1284bf603e4780c9dadd#diff-858b4452841160ed55e2d0a3a92ea51fR14
Completely baffled as to how this worked for you guys before!
I thought you had seen this in project.json for Castle Core 4.0:
"compile": {
"exclude": [
"Components.DictionaryAdapter.Tests/Xml/**/*",
"log4netIntegration/**/*"
]
}
https://github.com/castleproject/Core/blob/v4.0.0/src/Castle.Core.Tests/project.json#L45-L50
Rather than stuff around forever with the thousands of unit test files for DictionaryAdapter (which I want to move out of Castle Core anyway and will probably need .NET Standard 2.0 to really work well) I'd just do this for now for the .NET Standard 1.3 build. Since we have a single NLog version now you can exclude NLog too, however log4net should be restored for .NET Standard now that we are using log4net 2.0.8.
@jonorossi - Thanks. That confirms that I just need to pursue an exclude strategy for the netcoreapp framework moniker. Wheels are turning again :)
Have tests running now on dotnet core using NUnitLite with the console app method mentioned here: http://www.alteridem.net/2015/11/04/testing-net-core-using-nunit-3/. I still want to do a bit of tidy up before I submit the PR.
Was just wondering if you guys had any reservations with this approach? I tried the test adapters but I think they might be deprecated now and test discovery just does not seem to be working across any of the netcoreapp targets.
Look forward to your feedback.
If that is the only way to get NUnit tests to run with the new tooling on the .NET Core runtime then sure go for it, it doesn't sounds too bad. Do you know if that breaks the ReSharper test runner, or if it still picks up the tests in the executable project?
I agree it does look like the dotnet-test-xxx
test runners are now obsolete, xUnit.net only published a pre-release dotnet-xunit
.NET CLI tool package on April 6th, so NUnit is likely to be a while behind.
The project only builds via the command line(something I still want to investigate). Once you have it built, you can run the DesktopCLR tests via Resharper on a good day(they do tend to disappear every now and then). For debugging the dotnet core tests, you need to set Castle.Core.Tests as a startup project and hit F5. I can tweak it so you can target fixture/tests without having to run the entire suite(let's review this in the PR).
I am not going to lie to you, we are pushing VS2017 to it's outer limit here. It was never designed to do multi-targeting and even during builds it will kick out false negatives in terms of compiler errors. When you double click on the error and the code file opens, the error vanishes! Hopefully subsequent releases of Visual Studio 2017 will be better at handling this.
Once I have completed the log4net migration to core and tidied up a few lose ends I will submit a PR.
Thanks @jonorossi!
Sounds great, I look forward to taking a look.
Unfortunate to hear the new VS2017 .NET tooling isn't turning out as hoped, project.json really seemed like it was heading in a direction to support multi-targeting well 😞
If that is the only way to get NUnit tests to run
@jonorossi you had a xunit shim around for a while, would that be an option or too much trouble?
It was never designed to do multi-targeting and even during builds it will kick out false negatives in terms of compiler errors.
Would changing the Error List filter from Build + IntelliSense
to Build Only
help reducing the errors?
PR3
Just submitted PR: https://github.com/castleproject/Core/pull/252
It has rough edges, @jonorossi I am counting on your fastidious(rigorous, brutal) review here. I have preserved the TeamCity build even though we agreed we would not. I think the status checks are incredibly valuable before merging PR's. Let's take this to master and double up for now. We can alleviate this for Windsor to expedite jeremy and alina's work. I will be jumping down as soon as I can.
We have a bonus starting block for TravisCI. I tried building this on Ubuntu tonight and found that things very quickly went awry because of DesktopClr targetting. Given this project does not really support development workflows on Linux/Mac I think we are OK for now.
@jeremymeng - XUnit shim. We should avoid this(sorry Brad love your work). I think Rob Prowse has created an out
with NUnitLite. It also coalesces with Linux/Mac workflows(post project.json apocalypse). We should stay here until they work it out. I migrated all the tests for Windsor over to XUnit in fortress and I can honestly say it was not fun. Marshaling between the two concerns me that it would become a maintenance overhead that we don't care about which will sort itself eventually. Changing the error list filter was a great suggestion but these failures were in the build category, pull down the PR and take it for a spin running DesktopCLR tests via resharper and builds via the console while editing csproj files in between. Some very interesting non-deterministic feedback crops up :) Found a few of the issues you raised previously trying to get NUnit working for this project on netstandard/netcore, it was a massive help today!
We now have status checks passing for both appveyor and travisci :))))))
Raised an issue for the weird behaviour I saw earlier in this thread. https://github.com/castleproject/Core/issues/253
I wrote that NUnit shim over the xUnit.net library really because I had no option because at the time there was no NUnit library or runner for DNX previews, I really wouldn't want to resurrect that ugly thing if we've got the NUnitLite option. The NUnitLite option isn't too bad, just means you add a single line to your test project in static void Main and convert it to an EXE to become your test runner.
@fir3pho3nixx I thought the plan was to get all the functionality across from TeamCity into AppVeyor/Travis so we could retire TeamCity and do our release from the new scripts, and do that in this branch.
With regards to Travis, I'm more concerned about getting Mono support added than ensuring .NET Core works on it, I'm after a like-for-like with our TeamCity set up to know we haven't regressed from our current position, we haven't tested on .NET Core on Linux so that is just a bonus if that gets added not a required addition for this changeover.
Cool, will submit a PR for MONO on TravisCI next. 👍
Kaboom! Lots of little pieces down there :)
https://github.com/dotnet/sdk/issues/335
@jonorossi - Have a look at this. Guys are either upgrading to Mono 4.8 or moving over to standard. What's our next play here?
I'm happy for Mono to be upgraded to 4.8, I think that is the path of least resistance. Maybe in the future when the tooling improves we can get our unit tests to the point where everything passes on the Mono runtime, unfortunately some of our unit tests still crash the Mono runtime because of Mono bugs.
Investigated the Mono 4.8 a bit more. Anyone doing it successfully has not got a multi-targeting solution running on VS2017 builds for Linux/Mac. They are still using VS2015 csproj files which still translates well to the xbuild runner. MSBuild however are working on getting this going but it simply is not ready yet (https://github.com/Microsoft/msbuild see "mono, returning soon"). Their current Linux release build has stop errors on Linux(trusty) for VS2017.
The good news is we can delete Castle.Core-NetCore.sln and friends(.xproj/project.json et al). The bad news is we don't have the 'cut over' you were intending for @jonorossi (damn it, I was hoping for this too). We are still firmly locked in a VS2015(Mono)/VS2017(DesktopClr/NetStandard) scenario. You could see this as positive thing, all the windows stuff has been upgraded, no more project.json right? Honestly I miss that stuff, it was good!
I hate running side by side solutions, it sucks, a PR from one solutions perspective does not always pass for the other. You always have to keep the 2 in sync. I could offer a way of minimizing this by making the VS2015 stuff behave like the VS2017 stuff but that would require some MsBuild wizardry in the VS2015 old build layer. It would avoid things like files being "auto" included in VS2017 but causing compiler errors in the VS2015 project because nobody bothered to open that and include new files there. It is not a silver bullet though.
In summary, TravisCI wants xbuild/VS2015 for mono. Crapla.
The end game is status checks on PR's and opening this up to the internet. I think we should just go with to 'side-by-side' for now and abandon the cut over. Look forward to your thoughts @jonorossi ...
Edited
This was me naively hoping we could delete the old stuff. But it looks like we still need it. Groan
omnisharp-roslyn is using MSBuild from mono 4.8.0. Worth taking a look.
Yup, got this idea from here: https://github.com/nunit/nunit-console/pull/205/commits/a000037b2dfa698fc535e22face55bc97a897101
Except I get:
Unhandled Exception:
System.MissingMethodException: Method 'Array.Empty' not found.
at Microsoft.Build.CommandLine.MSBuildApp.Main () <0x40198d50 + 0x00073> in <filename unknown>:0
[ERROR] FATAL UNHANDLED EXCEPTION: System.MissingMethodException: Method 'Array.Empty' not found.
at Microsoft.Build.CommandLine.MSBuildApp.Main () <0x40198d50 + 0x00073> in <filename unknown>:0
My CAKE knowledge is also not where it should be BTW.
Edited
Will keep digging!
@fir3pho3nixx sounds like we should be able to move Mono to the new project files soon with the results coming from others projects. It is a little annoying keeping that old stuff, but I'm happy for it to stay for another few months if it isn't easy to get going right now.
I'm more interested about the cutover from TeamCity to AppVeyor/Travis, I'd like to shutdown the TeamCity VMs.
As you mentioned the TravisCI status check will fail if someone sends in a PR without adding the new file to the other project, except obvious new unit test classes, but we can deal with that we don't get a high volume of pull requests.
Before I give up, let me try and validate some of the problems I am seeing on another machine. Will ping back later with results.
I tried deconstructing the OmniSharp workflow for getting builds going using mono and found that the cake files for the OmniSharp roslyn project were downloading what appeared to be custom build artifacts for mono from an OmniSharp Azure storage location.
URL Here:
https://github.com/OmniSharp/omnisharp-roslyn/blob/dev/build.json#L6
Downloading Here:
https://github.com/OmniSharp/omnisharp-roslyn/blob/dev/build.cake#L137
So even though it is possible to get this going, I am not sure we want this.
I did pursue getting a development workflow going from Ubuntu Linux using VSCode which runs a cut down version of DotNetCore tests(strong naming was not applied, not sure if this is possible just yet). I then added this to the TravisCI build. Would love to get this going from OSX.
Strangely my PR is showing commits that have been previously merge as new ones :/
PR: https://github.com/castleproject/Core/pull/256
Look forward to feedback ...
I definitely agree that monkey patching Mono to get this to work isn't ideal and also a lot of work. We should either:
I'm leaning towards 2 so that our Mono support doesn't degrade as we make changes, it'll be annoying but hopefully shortlived. Thoughts?
Option 2 is most tenable for now. I am almost done with the TravisCI build (mono/netcoreapp1.1 for linux using the above mentioned method).
These are the build configurations we've currently have in TeamCity with the configuration pulled apart from the set up. It would be great if they all went to using a
build
script rather than calling msbuild/xbuild directly.I'm not sure exactly what we want to do but it might be time to move Castle Core to the new VS2017 tooling to support .NET Framework and .NET Core, this should hopefully make the NuGet packaging cleaner.
TeamCity: http://builds.castleproject.org/project.html?projectId=Core&tab=projectOverview AppVeyor: https://ci.appveyor.com/project/castleproject/core Travis CI: https://travis-ci.org/castleproject/Core
.NET 3.5
.NET 4.0
NET40-Release
configuration.NET 4.5
NET45-Release
configuration.NET Core
Mono (4.6.1)
Pack
A custom "Pack" build has two fields you can manually specify:
release_build
-checkbox checkedValue='true' description='Defines whether this build is intended for public release to NuGet (as either a prerelease or final version, rather than a CI build)' display='hidden' label='Release build' uncheckedValue='false'
version_suffix
-text description='Defines an optional version suffix (e.g. -beta001)' label='Version suffix' validationMode='any' display='hidden'