Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/coreclr/src/vm/corhost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ void SetCommandLineArgs(LPCWSTR pwzAssemblyPath, int argc, LPCWSTR* argv)
GCPROTECT_BEGIN(gc);

gc.cmdLineArgs = (PTRARRAYREF)AllocateObjectArray(argc + 1 /* arg[0] should be the exe name*/, g_pStringClass);
OBJECTREF orAssemblyPath = StringObject::NewString(pwzAssemblyPath);
OBJECTREF orAssemblyPath = StringObject::NewString(Bundle::AppIsBundle() ? static_cast<LPCWSTR>(Bundle::AppBundle->Path()) : pwzAssemblyPath);
gc.cmdLineArgs->SetAt(0, orAssemblyPath);

for (int i = 0; i < argc; ++i)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>$(NetCoreAppCurrent)</TargetFramework>
<OutputType>Exe</OutputType>
<RuntimeIdentifier>$(TestTargetRid)</RuntimeIdentifier>
<RuntimeFrameworkVersion>$(MNAVersion)</RuntimeFrameworkVersion>
</PropertyGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using System;

namespace EnvironmentGetCommandLineArgs
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
public class Program
{
public static void Main(string[] args)
{
foreach (var arg in Environment.GetCommandLineArgs())
{
Console.WriteLine(arg);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
using System;
using BundleTests.Helpers;
using Microsoft.DotNet.Cli.Build.Framework;
using Microsoft.DotNet.CoreSetup.Test;
using Xunit;

namespace AppHost.Bundle.Tests
{
public class BundleEnvironmentGetCommandLineArgs : IClassFixture<BundleEnvironmentGetCommandLineArgs.SharedTestState>
{
private SharedTestState sharedTestState;

public BundleEnvironmentGetCommandLineArgs(SharedTestState fixture)
{
sharedTestState = fixture;
}

[Fact]
public void GetEnvironmentArgs_0_Returns_Bundled_Executable_Path()
Comment thread
mateoatr marked this conversation as resolved.
{
var fixture = sharedTestState.TestFixture.Copy();
var singleFile = BundleHelper.BundleApp(fixture);

// For single-file, Environment.GetCommandLineArgs[0]
// should return the file path of the host.
Command.Create(singleFile)
.CaptureStdErr()
.CaptureStdOut()
.Execute()
.Should()
.Pass()
.And
.HaveStdOutContaining(singleFile);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mateoatr these tests should be separate facts. Thanks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

[Fact]
public void GetEnvironmentArgs_0_Non_Bundled_App()
{
var fixture = sharedTestState.TestFixture.Copy();
var dotnet = fixture.BuiltDotnet;
var appPath = BundleHelper.GetAppPath(fixture);

// For non single-file apps, Environment.GetCommandLineArgs[0]
// should return the file path of the managed entrypoint.
dotnet.Exec(appPath)
.CaptureStdErr()
.CaptureStdOut()
.Execute()
.Should()
.Pass()
.And
.HaveStdOutContaining(appPath);
}

public class SharedTestState : IDisposable
{
public TestProjectFixture TestFixture { get; set; }
public RepoDirectoriesProvider RepoDirectories { get; set; }

public SharedTestState()
{
RepoDirectories = new RepoDirectoriesProvider();
TestFixture = new TestProjectFixture("EnvironmentGetCommandLineArgs", RepoDirectories);
TestFixture
.EnsureRestoredForRid(TestFixture.CurrentRid, RepoDirectories.CorehostPackages)
.PublishProject(runtime: TestFixture.CurrentRid, outputDirectory: BundleHelper.GetPublishPath(TestFixture));
}

public void Dispose()
{
TestFixture.Dispose();
}
}
}
}