Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Add a test for PreInitData based on IL#3667

Merged
yizhang82 merged 4 commits intodotnet:masterfrom
yizhang82:preinittest
May 23, 2017
Merged

Add a test for PreInitData based on IL#3667
yizhang82 merged 4 commits intodotnet:masterfrom
yizhang82:preinittest

Conversation

@yizhang82
Copy link
Contributor

I checked under linux there is no ilasm in CLI so that needs a bit of additional work to plumb ILASM through CLI (at least the packages where we can locate ilasm). So this test is windows-only for now.

Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

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

I'm a bit concerned about our ability to keep the C# and IL file in sync. My preference for IL-based tests is to always trim the IL to the bare minimum so that it's understandable on it's own and maintainable by hand.

@@ -0,0 +1,357 @@

Copy link
Member

Choose a reason for hiding this comment

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

This might need a license header.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,9 @@
#!/usr/bin/env bash
Copy link
Member

Choose a reason for hiding this comment

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

I can't see this in the GitHub UI: does this file have the executable special mode flags (chmod)? Since it's not actually running, not setting them won't be a CI failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since I copied it from windows it didn't have the flags. I've fixed that.

// Code size 11 (0xb)
.maxstack 8
IL_0000: ldsfld native int [mscorlib]System.IntPtr::Zero
IL_0005: stsfld native int Details::PreInitializedField_DataBlob
Copy link
Member

Choose a reason for hiding this comment

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

This is an unintentional test of bad IL handling.

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. I don't need this cctor. Deleted.

@yizhang82 yizhang82 merged commit a806c3f into dotnet:master May 23, 2017
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