Return host's file path when using Environment.GetCommandLineArgs()[0] in single-file apps#41019
Conversation
|
Tagging subscribers to this area: @swaroop-sridhar, @agocke |
| @@ -0,0 +1,15 @@ | |||
| using System; | |||
|
|
|||
| namespace EnvironmentGetCommandLineArgs | |||
There was a problem hiding this comment.
I think it would be better to have one parametrized test asset for single-file API tests, rather than a different asset for each API. For example, #40974 will need a test for Assembly.CodeBase. We also need to add tests for Assembly.Location, AppContext.BaseDirectory, etc.
But I'm fine if we take this change, and adapt it when we test a second API.
vitek-karas
left a comment
vitek-karas
left a comment
There was a problem hiding this comment.
LGTM - regarding tests, I'm fine with this for now... I think we should revisit single-file tests after 5 to come up with some good way of doing it.
| .Pass() | ||
| .And | ||
| .HaveStdOutContaining(singleFile); | ||
|
|
There was a problem hiding this comment.
Hi @mateoatr these tests should be separate facts. Thanks.
There was a problem hiding this comment.
Does it make sense to have the second test here? Feels a little bit weird to test the behavior of a non single-file app in this file.
There was a problem hiding this comment.
I agree that this is not a great factoring. I suggested having the related test here because we have the infrastructure (and we cannot yet test single-file easily from Libraries subset). But its only a suggestion.
However, if we do keep the two versions they should be separate facts, because unit tests should test only one thing. Thanks for making the change.
Thanks for making the change.
|
/backport to release/5.0 |
|
Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/225028145 |
ghost
locked as resolved and limited conversation to collaborators
4 participants
Fixes #40874.