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

Fix execution of ilverify tests in outerloop runs #73132

Merged
merged 3 commits into from
Aug 2, 2022

Conversation

trylek
Copy link
Member

@trylek trylek commented Jul 31, 2022

As @jkotas noticed in #73109, outerloop tests don't run the
ilverify/ILVerificationTests.csproj test project. This is because
the project is somewhat atypical in residing just one directory
level beneath the test binary root folder; all other tests reside
under at least two directory levels and the legacy XUnit wrapper
generator reflects it so that each wrapper corresponds to
a two-level directory subtree.

The pre-existing logic for determining active two-level test
directories was quite hacky and perf-costly; I probably accidentally
removed the support for single-subfolder tests in one of my
refactorings of the src/tests/build.proj script. This change fixes
it and makes the construction of TestDirectories much more efficient
as we no longer construct a Carthesian product of all tests
times all two-level directories.

Thanks

Tomas

/cc @dotnet/runtime-infrastructure

@trylek
Copy link
Member Author

trylek commented Jul 31, 2022

/azp run runtime-coreclr outerloop

@ghost
Copy link

ghost commented Jul 31, 2022

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

As @jkotas noticed in #73109, outerloop tests don't run the
ilverify/ILVerificationTests.csproj test project. This is because
the project is somewhat atypical in residing just one directory
level beneath the test binary root folder; all other tests reside
under at least two directory levels and the legacy XUnit wrapper
generator reflects it so that each wrapper corresponds to
a two-level directory subtree.

The pre-existing logic for determining active two-level test
directories was quite hacky and perf-costly; I probably accidentally
removed the support for single-subfolder tests in one of my
refactorings of the src/tests/build.proj script. This change fixes
it and makes the construction of TestDirectories much more efficient
as we no longer construct a Carthesian product of all tests
times all two-level directories.

Thanks

Tomas

/cc @dotnet/runtime-infrastructure

Author: trylek
Assignees: -
Labels:

area-Infrastructure-coreclr

Milestone: -

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jkotas
Copy link
Member

jkotas commented Jul 31, 2022

/azp run runtime-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jkotas
Copy link
Member

jkotas commented Aug 1, 2022

The XUnitWrapper is failing to find the .cmd file to run the tests:

Microsoft.DotNet.XUnitConsoleRunner v2.5.0 (64-bit .NET 7.0.0-preview.5.22301.12)
  Discovering: windows.arm64.Checked.ilverify.XUnitWrapper (method display = ClassAndMethod, method display options = None)
  Discovered:  windows.arm64.Checked.ilverify.XUnitWrapper (found 1 test case)
  Starting:    windows.arm64.Checked.ilverify.XUnitWrapper (parallel test collections = on, max threads = 8)
    ilverify\ILVerificationTests.cmd [FAIL]
      Test Infrastructure Failure: System.ComponentModel.Win32Exception (2): An error occurred trying to start process 'D:\h\w\A67608B7\w\B15709AA\e\windows.arm64.Checked\ilverify\ILVerificationTests.cmd' with working directory 'D:\h\w\A67608B7\w\B15709AA\e'. The system cannot find the file specified.
         at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
         at CoreclrTestLib.CoreclrTestWrapperLib.RunTest(String executable, String outputFile, String errorFile, String category, String testBinaryBase, String outputDir)
         at windows_arm64_Checked_ilverify._ILVerificationTests_._ILVerificationTests_cmd()
      Expected: True
      Actual:   False
      Stack Trace:
           at windows_arm64_Checked_ilverify._ILVerificationTests_._ILVerificationTests_cmd()
           at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
           at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
  Finished:    windows.arm64.Checked.ilverify.XUnitWrapper

As JanK noticed in dotnet#73109, outerloop tests don't run the
ilverify/ILVerificationTests.csproj test project. This is because
the project is somewhat atypical in residing just one directory
level beneath the test binary root folder; all other tests reside
under at least two directory levels and the legacy XUnit wrapper
generator reflects it so that each wrapper corresponds to
a two-level directory subtree.

The pre-existing logic for determining active two-level test
directories was quite hacky and perf-costly; I probably accidentally
removed the support for single-subfolder tests in one of my
refactorings of the src/tests/build.proj script. This change fixes
it and makes the construction of TestDirectories much more efficient
as we no longer construct a Carthesian product of all tests
and all two-level folder directories.

Thanks

Tomas
@trylek
Copy link
Member Author

trylek commented Aug 1, 2022

/azp run runtime-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@trylek
Copy link
Member Author

trylek commented Aug 1, 2022

/azp run runtime-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@trylek
Copy link
Member Author

trylek commented Aug 2, 2022

The only remaining failure is the timeout in the llvmfullaot tests that is known and hopefully about to be addressed by #73265, merging in.

@trylek trylek merged commit eb4f246 into dotnet:main Aug 2, 2022
@trylek trylek deleted the RunILVerifyTests branch August 2, 2022 23:09
@jkotas
Copy link
Member

jkotas commented Aug 2, 2022

@trylek Thank you!

@ghost ghost locked as resolved and limited conversation to collaborators Sep 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants