Skip to content

Conversation

@natemcmaster
Copy link

Part of dotnet/aspnetcore#11169

This should help us ensure during patches that tests are running against the latest assemblies.

cc @HaoK

@natemcmaster natemcmaster requested review from a team, Tratcher and analogrelay as code owners June 13, 2019 01:48
@davidfowl
Copy link
Member

This is pretty terrible. Why do we need to do this? Can we do something that doesn't impact the dev experience?

@natemcmaster
Copy link
Author

The problem I’m trying to solve only happens during patching. To allow the repo to release a subset of packages, projects use package references to the major.minor.0 version when the repo is servicing.

So, scenario: we need to service Primitives in 3.0.1, but we don’t want to cascade the effect of this and reship Config. The resulting dependency graph is

Config.Tests —project ref—> Config (3.0.1) —pkg ref—> Config.Abstractions (3.0.0) —pkg ref—> Primitives (3.0.0)

Config.Tests runs against Primitives 3.0.0, not 3.0.1. This means we can’t be confident that the patch in Primitives 3.0.1 isn’t going to break something because we’re not testing the combination of latest packages.

Solutions:

  • we could ship every package on every patch instead of trying to incrementally patch the repo. So, Primitives 3.0.1 means we have to reship Config.Abstractions, Config, Logging, etc. the cascading effect.
  • we add a direct dependency from Config.Tests to Primitives. The reference resolution will resolve to a ProjectRef to Primitives (3.0.1)
  • we adopt a more powerful reference system like corefx’s.

@Tratcher
Copy link
Member

Tratcher commented Jun 13, 2019

Oh my. Is there any kind of tooling we can use to make sure this stays current? Or that we got all of them this time? Or does DisableTransitiveProjectReferences just make it not compile otherwise?

@HaoK
Copy link
Member

HaoK commented Jun 13, 2019

For the patch/release branches without this fix, the dev experience was basically a mirage since green PR checks weren't actually running against the actual coherent set of latest bits, that seems like a regression disaster waiting to happen. Until I understood what was going on, I actually 'fixed' tests in a patch PR to validate the wrong behavior due to mismatched bits.

Is it possible to limit the dev exp regression to only the servicing branches?

@natemcmaster
Copy link
Author

DisableTransitiveProjectReferences just make it not compile otherwise

Correct.

Is it possible to limit the dev exp regression to only the servicing branches?

Sure, but it means every time we finish a stable release we have to do this change, and then revert, or add this change in servicing only and don’t merge it back to master. Seemed cleaner to just always do this.

@natemcmaster
Copy link
Author

So, @davidfowl are you okay if we move forward with this? I'm okay if we consider alternatives in the long run, but this seemed like the least expensive short term fix we can make to ensure our servicing tests are more reliable.

@natemcmaster
Copy link
Author

I'm planning to merge this before I leave today. If there are better ideas later for how to solve the problem, I won't be offended if you revert.

FYI @JunTaoLuo

@natemcmaster natemcmaster merged commit e7d5bea into master Jun 13, 2019
@ghost ghost deleted the namc/transitive-tests branch June 13, 2019 23:27
@davidfowl
Copy link
Member

Yes it’s ok to leave for now

JunTaoLuo pushed a commit to dotnet/aspnetcore that referenced this pull request Feb 12, 2020
JunTaoLuo pushed a commit to dotnet/aspnetcore that referenced this pull request Feb 15, 2020
maryamariyan pushed a commit to maryamariyan/runtime that referenced this pull request Feb 28, 2020
maryamariyan pushed a commit to maryamariyan/runtime that referenced this pull request Mar 2, 2020
maryamariyan pushed a commit to maryamariyan/runtime that referenced this pull request Mar 11, 2020
maryamariyan pushed a commit to maryamariyan/runtime that referenced this pull request Mar 11, 2020
maryamariyan pushed a commit to maryamariyan/runtime that referenced this pull request Mar 27, 2020
maryamariyan pushed a commit to maryamariyan/performance that referenced this pull request Apr 3, 2020
maryamariyan pushed a commit to maryamariyan/runtime that referenced this pull request Apr 13, 2020
maryamariyan pushed a commit to maryamariyan/performance that referenced this pull request Apr 17, 2020
adamsitnik added a commit to dotnet/performance that referenced this pull request Jun 23, 2020
* Reorganize source code

In preparation for merging many other projects into this repo, this establishes a new source code organization which groups projects together based on subject matter.


Commit migrated from dotnet/extensions@7ce647c

* Merge branch 'release/2.1' into release/2.2


Commit migrated from dotnet/extensions@18fcffb

* Merge branch 'release/2.2'


Commit migrated from dotnet/extensions@34204b6

* Reorganize source code in preparation to move into aspnet/Extensions

Prior to reorganization, this source code was found in https://github.com/aspnet/DependencyInjection/tree/7a283947c231b6585c8ac95e653950b660f3da96


Commit migrated from dotnet/extensions@689c4a8

* Reorganize source code in preparation to move into aspnet/Extensions

Prior to reorganization, this source code was found in https://github.com/aspnet/DependencyInjection/tree/12eef5e86e965b9611221c72c169002e6f3038ee


Commit migrated from dotnet/extensions@04e957b

* Reorganize source code in preparation to move into aspnet/Extensions

Prior to reorganization, this source code was found in https://github.com/aspnet/DependencyInjection/tree/d77b090567a1e6ad9a5bb5fd05f4bdcf281d4185


Commit migrated from dotnet/extensions@9a4c61b

* Reorganize source code in preparation to move into aspnet/Extensions

Prior to reorganization, this source code was found in https://github.com/aspnet/Logging/tree/f7d8e4e0537eaab54dcf28c2b148b82688a3d62d


Commit migrated from dotnet/extensions@f3cd14a

* Reorganize source code in preparation to move into aspnet/Extensions

Prior to reorganization, this source code was found in https://github.com/aspnet/HttpClientFactory/tree/cf7cf83ee869ed50a41852f5e880d073386b7fe7


Commit migrated from dotnet/extensions@c38f9c1

* Use the SharedSourceRoot variable in project files


Commit migrated from dotnet/extensions@888bcba

* Allow caching of IEnumerable services (dotnet/extensions#575)



Commit migrated from dotnet/extensions@c89a5b5

* Reduce event source logger allocations (dotnet/extensions#870)



Commit migrated from dotnet/extensions@091ee13

* Use Arcade (dotnet/extensions#586)

Use arcade


Commit migrated from dotnet/extensions@f045899

* Cleanup conversion to Arcade (dotnet/extensions#1014)

* Remove obsolete targets, properties, and scripts
* Replace IsProductComponent with IsShipping
* Undo bad merge to version.props
* Update documentation, and put workarounds into a common file
* Replace usages of RepositoryRoot with RepoRoot
* Remove API baselines
* Remove unnecessary restore feeds and split workarounds into two files
* Enable PR checks on all branches, and disable autocancel


Commit migrated from dotnet/extensions@f41cfde

* Shrink StringValues (dotnet/extensions#1283)



Commit migrated from dotnet/extensions@2f8e1fb

* Disable transitive project references in test projects (dotnet/extensions#1834)



Commit migrated from dotnet/extensions@e7d5bea

* Fix HTTP client benchmarks


Commit migrated from dotnet/extensions@8f0f7fa

* Add typed client creation benchmark


Commit migrated from dotnet/extensions@20ce03a

* Support netcoreapp3.1 TFM (dotnet/extensions#2336)

* Support netcoreapp3.1 TFM

* Unpin SDK for source build

* Update to preview1 branding


Commit migrated from dotnet/extensions@32cc816

* Normalize all file headers to the expected Apache 2.0 license


Commit migrated from dotnet/extensions@cec6e75

* Switch file headers to the MIT license


Commit migrated from dotnet/extensions@321a30c

* Moves Microsoft.Extensions.* perf tests over

* Using the Open key to fully sign assembly

- Needed to later get InternalsVisibleTo on DI

* Apply suggestions from code review

Make batch commit using suggestions in PR feedback

Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>

* Fix compile + other feedback

* Apply suggestions from code review

Co-authored-by: Nate McMaster <nate.mcmaster@microsoft.com>
Co-authored-by: Pavel Krymets <pavel@krymets.com>
Co-authored-by: Ryan Brandenburg <rybrande@microsoft.com>
Co-authored-by: Nate McMaster <natemcmaster@users.noreply.github.com>
Co-authored-by: Ben Adams <thundercat@illyriad.co.uk>
Co-authored-by: Ryan Nowak <rynowak@microsoft.com>
Co-authored-by: John Luo <johluo@microsoft.com>
Co-authored-by: Sam Harwell <Sam.Harwell@microsoft.com>
Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>
JunTaoLuo pushed a commit to dotnet/aspnetcore that referenced this pull request Nov 17, 2020
@ghost ghost locked as resolved and limited conversation to collaborators May 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants