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

upgrade to Bullseye 2.0.0 RC2 #1456

Merged
merged 1 commit into from
Sep 27, 2018
Merged

upgrade to Bullseye 2.0.0 RC2 #1456

merged 1 commit into from
Sep 27, 2018

Conversation

adamralph
Copy link
Contributor

image

image

Exactly twice as fast with the new --parallel option! You may want to think twice before using it for CI though, since the console output from many targets get interleaved, and there's not much that can be done about that. I've been using it to speed up local builds.

@adamralph adamralph mentioned this pull request Sep 27, 2018
14 tasks
@blairconrad
Copy link
Member

Thanks, @adamralph. I'm not sure I'll be using it though. I blame it for

[xUnit.net 00:00:16.71]     FakeItEasy.IntegrationTests.TypeCatalogueTests.Should_warn_if_some_types_cannot_be_loaded_from_external_assembly [FAIL]
[xUnit.net 00:00:16.95]     FakeItEasy.IntegrationTests.TypeCatalogueTests.Should_be_able_to_get_types_from_fakeiteasy [FAIL]
[xUnit.net 00:00:16.96]     FakeItEasy.IntegrationTests.TypeCatalogueTests.Should_be_able_to_get_types_from_assembly_in_app_domain [FAIL]
[xUnit.net 00:00:16.96]     FakeItEasy.IntegrationTests.TypeCatalogueTests.Should_be_able_to_get_types_from_external_assembly [FAIL]
[xUnit.net 00:00:16.97]     FakeItEasy.IntegrationTests.TypeCatalogueTests.Should_warn_of_duplicate_input_assemblies_with_different_paths [FAIL]
[xUnit.net 00:00:16.97]     FakeItEasy.IntegrationTests.TypeCatalogueTests.Should_not_be_able_to_get_types_from_assembly_that_does_not_reference_fakeiteasy [FAIL]
[xUnit.net 00:00:16.98]     FakeItEasy.IntegrationTests.TypeCatalogueTests.Should_warn_of_bad_assembly_files [FAIL]
Failed   FakeItEasy.IntegrationTests.TypeCatalogueTests.Should_warn_if_some_types_cannot_be_loaded_from_external_assembly
Error Message:
 System.AggregateException : One or more errors occurred.
---- System.UnauthorizedAccessException : Access to the path 'FakeItEasy.ExtensionPoints.External.dll' is denied.

Unless we start generating framework-dependent external assemblies (which we could), it seems we're just asking for failures!

None of this means I'm against upgrading.

@blairconrad
Copy link
Member

blairconrad commented Sep 27, 2018

FWIW, I'm not wild about the interaction between -s and -p. Maybe I'm an oddball, but once I saw the parallel mode, I figured my favourite local build command would be build -s -p build spec integ unit. The -p means we'll run the tests at the same time as the build. I know, one could argue that by adding -s, I'm taking responsibility for the dependencies, and I got what I asked for. On the other hand, sometimes I'd rather have what I need, not what I ask for!

@adamralph
Copy link
Contributor Author

@blairconrad that's strange. It worked perfectly for me. That said, I probably wouldn't use --parallel for CI builds anyway. I see it as something for local builds only.

@blairconrad
Copy link
Member

Sure, @adamralph, but I was running local! And having to then rerun my build is a bit annoying. It may be an infrequent occurrence. Maybe exacerbated by my weird oldish laptop hardware. Then again, it has happened all one times that I've tried!

@blairconrad
Copy link
Member

blairconrad commented Sep 27, 2018

All two three times!

@adamralph
Copy link
Contributor Author

adamralph commented Sep 27, 2018

@blairconrad it sounds like what you really need is to run spec, integ, and unit, so the appropriate command would be build.cmd -p spec integ unit. Why are you putting build in there explicitly and using -s?

Regarding your failure, I guess some builds simply won't be able to use parallel target execution. If there are two targets contending for a resource, there's nothing Bullseye can do to remove that contention. Those targets will have to be changed before parallel execution can be used. It is weird that it works for me though. I've tried several times without failure.

@blairconrad
Copy link
Member

blairconrad commented Sep 27, 2018

@blairconrad it sounds like what you really need is to run spec, integ, and unit, so the appropriate command would be build.cmd -p spec integ unit. Why are you putting build in there explicitly and using -s?

Because I want to run tests on newly-built code, but I don't want the darn thing to bother with the gitversioning (and previously, restoring, which was the bigger deal). It's a small thing, but a thing. I'm probably better off running as you say. Except for the integs…

Regarding your failure, I guess some builds simply won't be able to use parallel target execution.

Absolutely. I didn't mean to suggest that it's a problem with the Bullseye feature. That's why I mentioned that I'm not against us upgrading.

It is weird that it works for me though. I've tried several times without failure.

Nondeterminism is like that sometimes (but not always! I kill me.). My hardware config at home must make me lucky. I just now tried on the work computer, and 3 successes right in a row, with about 50% reduction in execution time, as you saw. Which is very cool.

@adamralph
Copy link
Contributor Author

.\build.cmd build unit integ spec -n tells me that the only other target that will run is versionInfoFile, and when that is actually run, it only takes ~300ms, so adding -s to that command isn't really buying you much.

@blairconrad
Copy link
Member

~300 ms and a certain number of lines spewed to the screen. But yeah, it's not that big a deal. I'd trade the s for the -p if the latter worked.

I'd gotten in the habit of running .\build.cmd -s build unit integ spec back when there was a restore target, which took considerably longer, and even more spew.

@blairconrad
Copy link
Member

@adamralph, can we take the next RC instead?

@adamralph
Copy link
Contributor Author

@blairconrad absolutely. I've pushed the package, but NuGet is taking forever to index it. As soon as that's done, I'll push the amended commit.

@adamralph adamralph changed the title upgrade to Bullseye 2.0.0 RC1 [WIP] upgrade to Bullseye 2.0.0 RC2 Sep 27, 2018
@adamralph adamralph changed the title [WIP] upgrade to Bullseye 2.0.0 RC2 upgrade to Bullseye 2.0.0 RC2 Sep 27, 2018
@adamralph
Copy link
Contributor Author

I've pushed the amended commit. @blairconrad RC2 has the logging change we discussed.

@blairconrad
Copy link
Member

Thanks, @adamralph. I appreciate the custom RC2!

@blairconrad
Copy link
Member

This change has been released as part of FakeItEasy 4.9.1.

Thanks, @adamralph. Look for your name in the release notes! 🍵

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

Successfully merging this pull request may close these issues.

None yet

2 participants