Skip to content

Conversation

@bbartels
Copy link
Contributor

@bbartels bbartels commented Dec 26, 2020

Fixes #46236

The fix involves checking whether an interface method has a method body, as opposed to static, to avoid the search for an implementation in a child class.

I think the ILVerify building/testing workflow should be documented somewhere. It took me quite some time to figure out that the tests were only built when using priority=1.
Furthermore, I ran into the issue of running dotnet test in the ILVerify tests folder. This supposedly permanently breaks your build until you clean all artifacts and recompile (#43967).
Finding and fixing the issue took about half an hour, managing to get the tests working as a first time contributor to the ILVerify tool took about 4 hours.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the area-Tools-ILVerification Issues related to ilverify tool and IL verification in general label Dec 26, 2020
@bbartels bbartels changed the title Fix ILVerify Default Method Verification (#43967) Fix ILVerify Default Method Verification Dec 26, 2020
@am11
Copy link
Member

am11 commented Dec 26, 2020

managing to get the tests working as a first time contributor to the ILVerify tool took about 4 hours

Agreed, finding the solution for CS0234 error was not obvious, and took some time to figure out. I ended up with a little handrolled bash script to run a specific test after changing code and rebuilding the source and tests. Basically simulated this workflow for ILVerification project:

#### Running a single test on the command line

@bbartels
Copy link
Contributor Author

@am11
Honestly don't think I would have been able to resolve this without your issue. I don't have the most powerful PC so simply building .\build.cmd -c Release takes about 40 minutes. So I was very opposed to the idea of rebuilding if there was still a chance of fixing it without.

@bbartels
Copy link
Contributor Author

If I were to submit a PR to document the way to build this project should this be in docs/ or src/coreclr/tools/ilverify?
(Assuming this is desired in the first place)

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thank you!

@jkotas
Copy link
Member

jkotas commented Dec 26, 2020

If I were to submit a PR to document the way to build this project

This would be much appreciated!

hould this be in docs/ or src/coreclr/tools/ilverify

The documentation is in https://github.com/dotnet/runtime/blob/master/src/coreclr/tools/ILVerify/README.md currently. I think it would be best to keep updating this document with ILVerify specific testing instructions.

@bbartels
Copy link
Contributor Author

@jkotas should a separate PR be preferable or shall I append it to this one?

@jkotas jkotas merged commit 02fe1d7 into dotnet:master Dec 27, 2020
@jkotas
Copy link
Member

jkotas commented Dec 27, 2020

Could you please submit a new PR with the documentation changes?

@ghost ghost locked as resolved and limited conversation to collaborators Jan 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tools-ILVerification Issues related to ilverify tool and IL verification in general

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ILVerify rejects classes which inherit default interface method implementations

4 participants