-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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 a few special cases of projects with command line arguments #61758
Conversation
Tagging subscribers to this area: @hoyosjs Issue DetailsThis tackles the few special cases where command-line arguments are either ignored or used to run the test with a specific set of arguments; in the latter case I reworked the entry point to accept the parameters in its signature. This change doesn't address those cases where we run the same csproj test multiple times with different command-line combinations; that will require further design discussion with the GC team that owns most of the tests in this category. Thanks Tomas Contributes to: #54512
|
/azp run runtime-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
@@ -63,24 +63,24 @@ public static void Start2() | |||
genmeth2<string>(ninsts); | |||
} | |||
|
|||
public static int Main(String[] args) | |||
public static int Test(int threads, int insts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a case like this, would it make sense to keep the Main
that parses args and passes them to Test
, which you've created, but also add a test function that can be marked with [Fact]
to call it with the precise arguments the test case is using, e.g.,
[Fact]
public static int Test1()
{
return Test(4, 50);
}
the idea being, if you want to run the standalone test manually with arbitrary arguments, you still can, but the test harness calling the [Fact]
function will get the specific arguments.
Maybe for this one it doesn't matter, but I'm thinking maybe some of the JIT benchmarks might be like this.
(btw, unrelated, note that when you get to src\tests\JIT\Performance\CodeQuality
, those are "as close as we can make them" to the copies in the dotnet/performance repo at src\benchmarks\micro\runtime
)
Test entrypoint is parameterless and so it ignores the argument
The test just passes a pair of integers that are used as internal parameters for the test. Refactor the test so that the entrypoint accepts the parameters as arguments and call it once from Main the way we used to do based on the CLRTestExecutionArguments. Thanks Tomas
Based on PR feedback and Jeremy's check-in of the Roslyn generator test logic I can now fully update the test so that it no longer contains the Main method and it just implements the [Fact]-marked test entrypoint. Thanks Tomas
ff6b58f
to
1a558ef
Compare
Merging in with two unrelated bugs in library tests that are unaffected by this change. |
This tackles the few special cases where command-line arguments are either ignored or used to run the test with a specific set of arguments; in the latter case I reworked the entry point to accept the parameters in its signature. This change doesn't address those cases where we run the same csproj test multiple times with different command-line combinations; that will require further design discussion with the GC team that owns most of the tests in this category.
Thanks
Tomas
Contributes to: #54512