Reenable some helix tests #7763
Conversation
|
I think the dotnet runtime version upgrade (not part of this PR) might be causing some of the failures (it def exposed the issue with the env vars being setup wrong in the runtests.cmd) |
HaoK
force-pushed
the
haok/helix-uifix
branch
from
19b4c91 to
e31bb87
Compare
|
@dougbu can you take a quick look at a similar fix applied to the mvc analyzer test projects? |
HaoK
changed the title
Are you asking for a review of a PR? If so, yes. If you need something else, I don't have much time at the moment. |
|
Yes just rereview the PR, I added some changes to the MVC api test projects similar to the identity UI project, its just an additional publish + helix branch around where to find the test files |
|
@pakrym looks like the logging test errors are consistent on these tests now, I guess I will skip them for now, if you wanted to take a look at the logged tests that are failing with Null Ref, its these: https://mc.dot.net/#/user/aspnetcore/pr~2Faspnet~2Faspnetcore/ci/20190221.68 |
HaoK
changed the title
|
@pakrym finally things are looking good again with your logging fix, I put it in Directory.Build.targets for now since I don't have the rest of your refactoring in this PR. This is the best run I've seen in a while (passing ci and windows tests are all green in helix-test) so I'm thinking this is a candidate for squash/merge |
HaoK
force-pushed
the
haok/helix-uifix
branch
from
4a6b371 to
10cd0b5
Compare
HaoK
force-pushed
the
haok/helix-uifix
branch
from
10cd0b5 to
ddef1f8
Compare
src/Identity/test/Identity.Test/Microsoft.AspNetCore.Identity.Test.csproj
Outdated
Show resolved
Hide resolved
| @@ -148,13 +146,17 @@ private static List<ScriptTag> GetScriptTags(string cshtmlFile) | |||
| private static string GetSolutionDir() | |||
| { | |||
| var dir = new DirectoryInfo(AppContext.BaseDirectory); | |||
| while (dir != null) | |||
| // On helix we use the published copy | |||
| if (!string.Equals(Environment.GetEnvironmentVariable("helix"), "true", StringComparison.OrdinalIgnoreCase)) | |||
There was a problem hiding this comment.
I would rather prefer we had a shared way of doing this (looking for the test base directory). Rather then copying the snippet around.
There was a problem hiding this comment.
Do you mean add a test helper method for this that both the helix skip attribute and actual tests can use to check if we are running on helix?
There was a problem hiding this comment.
Add a way to find the test asset root. Or at minimum to detect running in helix.
There was a problem hiding this comment.
I don't feel comfortable generalizing a single test, I can add the helix detection now since the HelixSkip has the same logic. Once I fix a few more tests and see a clear pattern we can look into a generalized shared GetSolutionDir.
There was a problem hiding this comment.
@pakrym added a static helper method for now which encapsulates the Environment helix check
HaoK
changed the title
Publishes the src files the tests is using so they get included as part of the helix payload
@ajcvickers @natemcmaster