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

Set RollForward=Major for test projects #6160

Closed
wants to merge 1 commit into from

Conversation

dagood
Copy link
Member

@dagood dagood commented Sep 11, 2020

This lets them run locally without manually installing extra runtimes. #6126

I still get some different test failures locally after doing this, but I'm curious if the RollForward change will break PR validation in some other way.

@dagood dagood self-assigned this Sep 11, 2020
@riarenas
Copy link
Member

Thanks Davis for looking into it. I wonder if adding the 2.1 runtime to the global.json so that arcade installs it during the build might be a narrower fix?https://github.com/dotnet/arcade/blob/cad8f9fea5cc39501144890ab37d187d3adbdcdf/Documentation/ArcadeSdk.md#example-restoring-multiple-net-core-runtimes-for-running-tests

@dagood
Copy link
Member Author

dagood commented Sep 11, 2020

It's a different fix, and maybe more along the lines of what the infra expects. It's true that technically by running on a newer runtime, the tests could depend on some behavior that only works in the new runtime. It's also potentially odd, though, that we're testing (only?) against an old runtime even though the build tasks will be running against the newer runtime in everyone's repos. I don't really have expertise here, up to you if this is something dotnet/arcade should support and how.

@alexperovich
Copy link
Member

The tests run with --roll-forward Major in helix, so I think this change is better because it makes the behavior the same in both places. I also agree that we should be testing on the newer runtime because all the tasks run on the newer runtime.

@dagood dagood marked this pull request as ready for review September 11, 2020 17:27
@ViktorHofer
Copy link
Member

Why not instead apply a rollforward policy in global.json that is repo wide like we do in dotnet/runtime?

@dagood
Copy link
Member Author

dagood commented Sep 15, 2020

That sounds fine to me, too. Didn't know you could do it that way.

@ViktorHofer
Copy link
Member

ViktorHofer commented Sep 16, 2020

Cool. Yes, I would prefer the global.json setting over the msbuild property.

Base automatically changed from master to main March 8, 2021 23:09
@jonfortescue
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ChadNedzlek
Copy link
Member

So this is over a year old. Are we still working on this, or can it be closed?

@dagood
Copy link
Member Author

dagood commented Oct 4, 2021

Are we still working on this, or can it be closed?

I'm not. (If anyone wants to pick it up, note that #6126 is more broadly about tests not working locally on Linux, not just the runtime expectation.)

@dagood dagood closed this Oct 4, 2021
@dagood dagood deleted the roll-forward branch October 4, 2021 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants