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

Return empty string in InternalAssemblyBuilder.Location #57396

Merged
merged 7 commits into from
Aug 20, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public override FileStream[] GetFiles(bool getResourceModules)
throw new NotSupportedException(SR.NotSupported_DynamicAssembly);
}

public override string Location => throw new NotSupportedException(SR.NotSupported_DynamicAssembly);
public override string Location => string.Empty;

[RequiresAssemblyFiles(ThrowingMessageInRAF)]
public override string? CodeBase => throw new NotSupportedException(SR.NotSupported_DynamicAssembly);
Expand Down
13 changes: 11 additions & 2 deletions src/libraries/System.Reflection.Emit/tests/AssemblyBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.Collections.Generic;
using System.Linq;
using System.Runtime.InteropServices;
using Xunit;

namespace System.Reflection.Emit.Tests
Expand Down Expand Up @@ -424,7 +423,17 @@ void Invoke_Private_SameAssembly_ThrowsMethodAccessException()
Assert.Throws<MethodAccessException>(() => d ());
}


[Fact]
public void DefineDynamicAssembly_InternalAssemblyLocationIsEmpty()
{
AssemblyBuilder assembly = Helpers.DynamicAssembly();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
AssemblyBuilder assembly = Helpers.DynamicAssembly();
AssemblyBuilder assembly = Helpers.DynamicAssembly(nameof(DefineDynamicAssembly_InternalAssemblyLocationIsEmpty));

Otherwise the assembly will get a default name TestAssembly and the test might succeed by accident if some other test uses the same assembly name. (Unlikely as the test would still likely do the right validation, but still)

object internalAssemblyBuilder = assembly.GetType()
.GetProperty("InternalAssembly", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(assembly);
object locationObj = internalAssemblyBuilder.GetType().GetProperty("Location", BindingFlags.Public | BindingFlags.Instance)
.GetValue(internalAssemblyBuilder);
string location = Assert.IsType<string>(locationObj);
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to rewrite the test to not use private reflection?
The test is valid regardless of how the builder is implemented, but now it would fail if we changed the implementation (to rename/remove the internal property).
I think it should be possible to call something like AssemblyLoadContext.Default.Assemblies and find the one just defined and validate it there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the clue

Assert.Empty(location);
}

}
}