Skip to content

Conversation

@dougbu
Copy link
Contributor

@dougbu dougbu commented Jun 8, 2022

- see #41937
- do not skip `SimpleWebSiteWithWebApplicationBuilder.DefaultEnvironment_Is_Development()`
- get debug view of server's configuration when that test is going to fail
@ghost ghost added this to the 6.0.x milestone Jun 8, 2022
@ghost
Copy link

ghost commented Jun 8, 2022

Hi @dougbu. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@dougbu
Copy link
Contributor Author

dougbu commented Jun 8, 2022

/fyi @javiercn @HaoK @MackinnonBuck @dotnet/aspnet-build

I'm going to run this until DefaultEnvironment_Is_Development() fails then look at the extra output…

@dougbu
Copy link
Contributor Author

dougbu commented Jun 8, 2022

Got it in one. I think the important bit of the console log is

ENVIRONMENT=Production (CommandLineConfigurationProvider)

I do see --environment=Production on the faked command line for the test web site, probably due to

Also see the following

Could be something expecting launchSettings.json to work (though it's not copied to the bin/ or publish/ folders), issues setting or reading environment variables, a new issue in configuration precedence (@ericstj have there been any changes in that area❔), or something I don't have enough imagination to speculate about.

The relevant code on our side hasn't changed since early May but I may be missing something. The problem doesn't reproduce (at least that I've seen) when testing locally in VS and using dotnet test.

Oddly, problems are not consistent. We were not completely on the floor before we started skipping the tests but it was close.


@halter73 could this be a delayed impact of fd6ea53 or 4279611


@ericstj the general background is template tests and the Mvc test I'm messing w/ in this PR started failing about a week ago in both release/6.0 and main. #41937 doesn't contain much information and this PR is an attempt to root cause the problem. Common symptom in all failing tests mentioned in the issue is a Production environment when Development is expected. (The Mvc test just happens to be the simplest failing test but it still uses our TestHost, MvcTestFixture, and more.)

@Pilchie Pilchie added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Jun 8, 2022
@ghost
Copy link

ghost commented Jun 8, 2022

Hey @dotnet/aspnet-build, looks like this PR is something you want to take a look at.

@dougbu
Copy link
Contributor Author

dougbu commented Jun 8, 2022

I think the general question here is "where did the Development setting come from in the past❔" Then, we can figure out why that's working locally but not in the CI.

I'm going to do a few Mvc functional test runs locally to see if problems reproduce when including everything. My tests yesterday used dotnet test --no-build --filter "FullyQualifiedName~SimpleWithWebApplicationBuilderTests.DefaultEnvironment_Is_Development" and dotnet vstest Microsoft.AspNetCore.Mvc.FunctionalTests.dll /TestCaseFilter:"FullyQualifiedName~DefaultEnvironment_Is_Development" i.e. just ran one test.

@dougbu dougbu added the * NO MERGE * Do not merge this PR as long as this label is present. label Jun 8, 2022
@HaoK
Copy link
Member

HaoK commented Jun 8, 2022

@dougbu I think this is probably a subtle regression in hosting at this point, perhaps @halter73 or @captainsafia should investigate where the regression is, but its likely a product bug that's been uncovered here

@javiercn
Copy link
Member

javiercn commented Jun 9, 2022

I think the general question here is "where did the Development setting come from in the past❔" Then, we can figure out why that's working locally but not in the CI.

The template tests set it on an environment variable.

The WebApplicationFactory sets it by default too via code.

Something on the Helix environment seems to be overriding that, which is why I was trying to get the debug config listed out on the test. Unfortunately, the test didn't seem to capture the logs.

It might be one or two things:

  • Something in hosting changed that made the environments not apply (I believe the tests run fine locally, don't they? (According to @MackinnonBuck)) or apply in a different order.
  • Something on the Helix environment changed that is overriding the defaults.

@MackinnonBuck
Copy link
Member

Confirming that the tests passed locally for me.

@dougbu
Copy link
Contributor Author

dougbu commented Jun 9, 2022

Unfortunately, the test didn't seem to capture the logs.

We did get logs in this case. See https://dev.azure.com/dnceng/public/_build/results?buildId=1812519&view=ms.vss-test-web.build-test-results-tab&runId=48177922&resultId=118933&paneView=attachments and my comment above indicating the environment value came from the command line.

@dougbu
Copy link
Contributor Author

dougbu commented Jun 9, 2022

Confirming that the tests passed locally for me.

My local tests also passed consistently (ignoring #29597 rearing its ugly head when I used dotnet vstest in the publish/ folder)

@dougbu
Copy link
Contributor Author

dougbu commented Jun 9, 2022

Something in hosting changed that made the environments not apply

Possible but the code is just _config[key] = value;. Breaking that would be difficult.

apply in a different order.

This seems more likely to me, especially because 4279611 added another ConfigurationManager to the mix. Thoughts @halter73

Overall, still really weird the problem doesn't repro locally and it's using the command-line setting, not (say) finding something leftover in environment variables. That said I missed fact ASPNETCORE_ENVIRONMENT is also set on the Helix agent:

ASPNETCORE_ENVIRONMENT=Production (EnvironmentVariablesConfigurationProvider Prefix: '')

Should that "win" even though the environment is explicitly set to Development in code❔ I am also wondering where that comes from since we only set the environment variable to Production in IIS tests (StartupTests in particular). launchSettings.json files all use Development.

@wtgodbe
Copy link
Member

wtgodbe commented Jun 9, 2022

@dougbu do you need to preserve anything from this CI job, or should I re-kick it now that #42115 is in?

@dougbu
Copy link
Contributor Author

dougbu commented Jun 9, 2022

I've been leaving this alone since the aim was to hit the environment problem, not to get things green. Let's continue that until we have an idea of a fix to try out…

@dougbu
Copy link
Contributor Author

dougbu commented Jun 10, 2022

There was a Helix agent change (from Windows 11 Preview to Windows 11 RTM) around the time of the first failure (in #20220527.1).

However, that first failing build was a scheduled build of aspnetcore-helix-matrix, introducing a couple of different Windows queues to the mix (Windows.Amd64.Server2022.Open and Windows.10.Amd64.Server20H2.Open). And neither of those queues was likely affected by @MattGal's change. Right @MattGal

I don't see anything much in the recent history of 'main' but I guess something somewhere could now be leaving ASPNETCORE_ENVIRONMENT behind, probably from one of our other work items (pretty consistently). To be the root cause, we'd also need a bug somewhere in what ((IConfigurationRoot)config).GetDebugView() shows.

No, I don't know exactly how the Helix SDK orders work items on agents. Doubt we've changed how we order things in our submissions, especially not in release/6.0.

- unset `ASPNETCORE_ENVIRONMENT`
@dougbu
Copy link
Contributor Author

dougbu commented Jun 10, 2022

/azp run aspnetcore-helix-matrix

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@dougbu
Copy link
Contributor Author

dougbu commented Jun 10, 2022

/azp run aspnetcore-ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MattGal
Copy link
Member

MattGal commented Jun 10, 2022

And neither of those queues was likely affected by @MattGal's change. Right @MattGal

Server 2022 is the server equivalent of Windows 11, so if something interesting did happen in the client around your environment variables it could have happened there too. I still feel strongly that this is likely just some stateful change made unintentionally by a previous work item on the same machine though.

@halter73
Copy link
Member

halter73 commented Jun 10, 2022

@halter73 could this be a delayed impact of fd6ea53 or 4279611

No way for fd6ea53. Possibly for 4279611, but that was merged back in January for 6.0.4. We've already shipped 6.0.5. Delayed indeed.

@dougbu
Copy link
Contributor Author

dougbu commented Jun 10, 2022

Closing this out because the workaround seemed to work and #41939 and #41940 are up.

@dougbu dougbu closed this Jun 10, 2022
@dougbu dougbu deleted the dougbu/debug.41937 branch June 10, 2022 23:25
@dougbu dougbu removed this from the 6.0.x milestone Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework investigate * NO MERGE * Do not merge this PR as long as this label is present.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants