Skip to content
This repository has been archived by the owner on Jan 24, 2021. It is now read-only.

Update testing infrastructure #2612

Closed
19 of 20 tasks
thecodejunkie opened this issue Nov 13, 2016 · 31 comments
Closed
19 of 20 tasks

Update testing infrastructure #2612

thecodejunkie opened this issue Nov 13, 2016 · 31 comments

Comments

@thecodejunkie
Copy link
Member

thecodejunkie commented Nov 13, 2016

Description

We need to overhaul the test infrastructure, in the Nancy test projects, and make sure they all run the same version of xunit. We also need to bump the used version of FakeItEasy and re-enable all our tests on .NET Core.

Pre-requisits

Complete the migrations detailed in #2606 "Migrate remaining projects to support netstandard 1.6"

Required steps

  1. Update the xunit dependency in all packages.config to version 2.2.0-beta2-build3300 (this it the version we currently use in `project.json)
  2. Ensure all project.json really use 2.2.0-beta2-build3300
  3. Update all Nancy.*.Tests projects to use FakeItEasy 2.3.1 (waiting for confirmation by @adamralph that this is the version we need for .NET Core)
  4. Re-enable all tests for .NET Core

ℹ️ There is no need to bump the version of dotnet-test-xunit, for project.json, because we are already running the latest (2.2.0-preview2-build1029)

TODO

(up this list and link any pull-requests that are sent to address the above)

ℹ️ Only send a pull-requests once you have confirmed that all the required steps (above) have been satisfied

Valid rejection status for each of the projects

  • Up-to-date
  • 3rd-party dependency does not support .NET Core
  • Not supported on .NET Core

Example

  • Nancy.Foo (Not supported on .NET Core)
  • Nancy.Bar (Up-to-date)
  • Nancy.Baz (pull-requests #9999)
@adamralph
Copy link
Contributor

/cc @blairconrad @thomaslevesque

@thecodejunkie thecodejunkie changed the title [WIP] Update testing infrastructure Update testing infrastructure Nov 13, 2016
@blairconrad
Copy link
Contributor

Update all Nancy.*.Tests projects to use FakeItEasy 2.3.1

@thecodejunkie, the FakeItEasy packages on NuGet.org do not support .NET Core. The packages that target .NET Standard 1.6 can only be had from our AppVeyor feed.
As I type

  • FakeItEasy 2.4.0-netstd-build000034 and
  • FakeItEasy.Analyzer 2.4.0-netstd-build000034

are available. I think they'll meet your needs.

@thecodejunkie
Copy link
Member Author

@blairconrad thanks for the update. Do you have an ETA of the packages on NuGet.org? Ideally I'd like to avoid yet another package source, unless it's months away

@blairconrad
Copy link
Contributor

@thecodejunkie, unfortunately we do not.
We won't be able to publish "production" versions until Castle.Core releases a non-beta version, and it's not clear when that's coming.
There are also a few small things that we should sort out internally before we release a production version, such as (minor) differences in the available functionality in the .NET Core and .NET Regular FakeItEasy libraries. Also, we currently only have unit tests and API-conformance tests available for the .NET Core build: no integration or acceptance tests.

I understand your reluctance to add another package source, but had I to suggest a path today, I'd add it.

@thecodejunkie
Copy link
Member Author

@blairconrad after a short twitter exchange with @jonorossi it seems they were waiting on FakeItEasy, while you have been waiting on them. Maybe we can resolve this dead-lock? 😄

@jonorossi
Copy link
Contributor

until Castle.Core releases a non-beta version, and it's not clear when that's coming.

@blairconrad I had no idea you guys were waiting on a "stable" release, I was actually waiting on the 3 mocking libraries to have betas on nuget.org before going with a final release, especially that we made changes in beta002 for FakeItEasy. How did the AdditionalAttributes changes work out, did everything work well.

This is actually the first time someone has asked me for an non-prerelease build.

@jonorossi
Copy link
Contributor

When I say a "beta" on nuget.org, I just meant any pre-release end users could try things out, especially since the Castle.Core package dependency gets exposed rather than ILMerged on .NET Core. Moq and NSubstitute have had alpha/beta/rc for a while.

@jonorossi
Copy link
Contributor

@blairconrad there wasn't really a discussion anywhere around this since I'm the only one driving the Castle ship, I've commented on the last remaining issue about deferring it until post-v4, but got no response so no one watching Castle Core had an opinion.

I just thought it was a good idea to make sure all 3 mocking frameworks ship fully working in case we need to make breaking changes like beta002. If you want me to skip ahead without a FakeItEasy pre-release on nuget.org, then I'm okay with that.

@blairconrad
Copy link
Contributor

@jonorossi, I didn't comment on castleproject/Core#201 because I have no interest in the log4net integration, so I didn't want to butt in.

Thanks for thinking of us mockers. The FakeItEasy owners have just had a conversation and we're going to work on a 3.0-prerelease (with .NET Standard support) for NuGet.org.

We don't have a firm date (of course), but it should be soon enough that @thecodejunkie can wait to use the NuGet.org package pre-release package, and also for @jonorossi to wait on it for the Castle.Core 4.0 release. That is days, not weeks, before the pre-release package appears on NuGet.org.

@jonorossi
Copy link
Contributor

I didn't comment on castleproject/Core#201 because I have no interest in the log4net integration, so I didn't want to butt in.

That issue was never in the v4 milestone, I assume you were thinking about the NLog not having a non-prerelease package one (castleproject/Core#200). We'll just downgrade NLog and remove .NET Core to ship Castle Core with only the Serilog adapter working on .NET Core, until NLog and log4net get their releases sorted.

That is days, not weeks, before the pre-release package appears on NuGet.org.

Great, I'd be happy to cut the 4.0.0 final once Nancy has updated to that package, would be a good test with such a large codebase verifying FakeItEasy.

I know it isn't the best way to run an OSS project but don't assume I've actually got plans for Castle releases, everything has been reactive based on what people want, just log an issue or ping me if you guys need something, I'll do my best to find the time to get what you need done.

@blairconrad
Copy link
Contributor

Yeah, we don't map out the releases either.
Fix bugs as reported (when we can), maybe work on a feature that looks fun when it occurs to us. Release whenever (preferably soon). Were it not for the desire to make breaking changes somewhat infrequently, I think there'd be no plan at all!

Thanks for the offer, and your continued work!

@blairconrad
Copy link
Contributor

@jonorossi, @jchannon, in case you missed it, FakeItEasy 3.0.0-alpha001 is out. I've done no testing outside our usual release process, but if I find time today, will see if I can hook it up to NancyFx. Or vice versa.

https://github.com/FakeItEasy/FakeItEasy/releases/tag/3.0.0-alpha001

@blairconrad
Copy link
Contributor

I'm still asleep. Don't know why I put @jchannon in there when I meant @thecodejunkie. Apologies to whichever of you is most offended.

@jchannon
Copy link
Member

I am offended beyond belief sir. I will see you at dawn for a dual.

On Sun, 20 Nov 2016 at 13:22, Blair Conrad notifications@github.com wrote:

I'm still asleep. Don't know why I put @jchannon
https://github.com/jchannon in there when I meant @thecodejunkie
https://github.com/thecodejunkie. Apologies to whichever of you is most
offended.


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#2612 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAGapgYDfUwV8o6SnVJBqrNWgEn4QqNHks5rAEmWgaJpZM4KwsnS
.

@adamralph
Copy link
Contributor

A duel would be more traditional.

@jchannon
Copy link
Member

I blame the Github UI it cut off "carriageway drag race"

On 20 November 2016 at 19:27, Adam Ralph notifications@github.com wrote:

A duel would be more traditional.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2612 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAGapiPrpgurJ3EIyuErGNna_cSDBXQbks5rAJ8pgaJpZM4KwsnS
.

@adamralph
Copy link
Contributor

👍 makes sense. I was thinking that you probably meant that, being more progressive than traditional.

@blairconrad
Copy link
Contributor

Unless I'm misinterpreting, this issue is "grab a project, add a more modern .NET Core-supporting framework section to it, and make the darn thing run", no?
I can't promise fast progress, and I've made some extremely dubious changes, but for netcoreapp1.0, I've accomplished

=== TEST EXECUTION SUMMARY ===
Nancy.Tests  Total: 1814, Errors: 0, Failed: 1, Skipped: 0, Time: 40.687s

Which I feel is somewhat encouraging. If I'm actually doing something that's desired, I'll whip up a super rough PR in (my) morning.

@blairconrad
Copy link
Contributor

An update: FakeItEasy-3.0.0-beta002 has been released to NuGet.
I use it in #2634, which passes, including running the Nancy.Tests tests under netcoreapp1.0 on both Windows and Unix.

@NancyFx/owners, I know everyone's busy, and I don't mean to push, but is there something I've missed before the PR can be reviewed? (I am convinced you'll want changes, which I'm happy to make.)

@jonorossi, I think that even without the changes to the PR, we've an indication that FakeItEasy can perform well in the context of the project. Does provide enough support for you to go ahead with the Castle.Core 4.0.0 release?

@jonorossi
Copy link
Contributor

@blairconrad yep thanks, will get it sorted in the next couple of days.

@blairconrad
Copy link
Contributor

blairconrad commented Dec 11, 2016

Thanks, @jonorossi. No particular rush, but I wanted to know whether I was holding things up. 🐳

@jonorossi
Copy link
Contributor

Apologies for the delay, Castle Core 4.0.0 final is available: http://www.castleproject.org/blog/2017/01/25/core-4.0.0-release/

/cc @blairconrad, @thomaslevesque

@thomaslevesque
Copy link

Apologies for the delay, Castle Core 4.0.0 final is available: http://www.castleproject.org/blog/2017/01/25/core-4.0.0-release/

Awesome, thanks @jonorossi!

@blairconrad
Copy link
Contributor

Yes! Thanks, @jonorossi!

@thecodejunkie
Copy link
Member Author

Sorry for dropping the ball on this (work/life stuff). What is needed from Nancy. Code review? Anything else? Can we perhaps create a checklist in #2634 of things that are needed so we can start ripping into it? 😍

@thecodejunkie thecodejunkie self-assigned this Jan 24, 2017
@blairconrad
Copy link
Contributor

@thecodejunkie, work and life happen. Don't worry. Assuming I've understood this whole issue, I think the next step is a review of #2634. I've surely done some things that are not correct or in keeping with Nancy's style. Where I'm aware of them, I've made notes in the review. I'm happy to change anything - no comment is too petty! But until I get some feedback from Team Nancy, I don't know what to adjust.

@blairconrad
Copy link
Contributor

#2634 (I think) qualifies to check the "Nancy.Tests" box way upstairs.
And I've begun #2705 for "Nancy.Authentication.Basic.Tests".

@blairconrad
Copy link
Contributor

blairconrad commented Mar 8, 2017

Assuming #2713 is acceptable, I think the list upstairs could be updated to something like the following (I did not trace down whether components' 3rd-party requirements supported .NET Core or not; if the production project didn't have a .NET Standard project, I just stopped there).

The only outstanding item that I can see is running the .NET Core App Nancy.Embedded.Tests on Unix, and I have hit a wall there.

@horsdal
Copy link
Member

horsdal commented Mar 8, 2017

Updated described from @blairconrad comment

@thecodejunkie
Copy link
Member Author

@horsdal you just beat me to merging #2713 and now updating this list! 👍

@blairconrad super thanks!

@blairconrad
Copy link
Contributor

@thecodejunkie, a pleasure.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants