shana / google-highly-open-participation-mono

Automatically exported from code.google.com/p/google-highly-open-participation-mono
0 stars 0 forks source link

Improve System.Reflection.Emit test suite for generics #17

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Mono's test suite for System.Reflection.Emit is small and it gets really
bad when it comes to generics.

It would be really really nice to have tests for the encoding of generic
types on IL when the code is written to an assembly.

The tests should be really simples, just build some types, write them to
disc, load and check if the code execute correctly. 

This task is doable is 5-7 days, it will take a lot of reading time but the
coding time is quite small.

We already have some tests under mcs/class/corlib/Test/System.Reflection.Emit/

Original issue reported on code.google.com by kump...@gmail.com on 27 Nov 2007 at 1:44

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I claim this task.

Also, I'd like to clarify, do these have to be NUnit tests? And how many tests 
should I make, as a rough estimate?

Original comment by AdT...@gmail.com on 4 Jan 2008 at 2:40

GoogleCodeExporter commented 9 years ago
Hi AdTsai.

The tests should be using NUnit. Version 2.2.0.0 is desirable, since it's the 
version
mono uses, but not required.

First, you need to take a look at mcs/class/corlib/Test/System.Reflection.Emit/ 
to
see what tests we have.

A starting point could be the classes GenericTypeParameterBuilder, 
LocalBuilder, 
ParameterBuilder and SignatureHelper, which don't have any tests. Beware that
GenericTypeParameterBuilder defines only a few methods and the vast majority 
comes
from inheritance.

Other types, like TypeBuilder, FieldBuilder and MethodBuilder needs more tests 
for
validation of calls and creation.

A good amount of tests would be about 3 for each public method of
GenericTypeParameterBuilder, LocalBuilder, ParameterBuilder and 
SignatureHelper. Only
methods that matters should be tested, no need to do LocalBuilder::GetHashCode, 
for
example.

A good bonus would be testing TypeBuilder, FieldBuilder and MethodBuilder, they 
need
more tests for scenarios that creation should fails. These are harder to write 
as it
requires more experimentation with the API.

Original comment by kump...@gmail.com on 5 Jan 2008 at 2:20

GoogleCodeExporter commented 9 years ago

Original comment by kump...@gmail.com on 5 Jan 2008 at 2:24

GoogleCodeExporter commented 9 years ago
I've done the tests for the GenericTypeParameterBuilder. It builds a bunch of 
types 
with generic parameters, saves them to an assembly, then reloads it from disk. 
The 
tests themselves just check that the loaded types are intact and all 
constraints, 
etc. are working.

All tests have been compiled and tested against the Microsoft .NET Framework 
2.0, 
and all tests are confirmed to be working under the MS implementation. 

Original comment by AdT...@gmail.com on 7 Jan 2008 at 5:46

Attachments:

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I've been thinking of a way to test the LocalBuilder class. I need to test the 
LocalBuilder class using generics, by writing the assembly out to disk and then 
reloading it to test if everything still works, correct? My first thought was 
to 
define a method using the ILGenerator to emit opcodes to use the locals 
dependant on 
generic types. Then write the method to disk, reload it, and see if it still 
executes correctly. But it seems incredibly complicated for something so 
simple. Any 
ideas?

Original comment by AdT...@gmail.com on 7 Jan 2008 at 5:48

GoogleCodeExporter commented 9 years ago
AdTSai, you only need to test LocalBuilder::SetLocalSymInfo two overloads.

You can create an assembly with one type and one method, then stuff some locals 
on
that method which you called SetLocalSymInfo. To test the result, load the 
result
assembly and check the values using reflection.

I'll be reviewing your first patch tomorrow.

Thanks!

Original comment by kump...@gmail.com on 7 Jan 2008 at 9:42

GoogleCodeExporter commented 9 years ago
Yes, but I can't for the life of me figure out how to access symbolic 
information to 
confirm that it is correct. I've defined a type, with a method, with a bunch of 
locals. The locals work fine, but I don't have any idea whether the symbolic 
information specified by LocalBuilder.SetLocalSymInfo() is actually working or 
not. 
I can't find anything in the Reflection libraries which exposes an interface to 
view 
symbolic information from an assembly.

Original comment by AdT...@gmail.com on 8 Jan 2008 at 6:50

GoogleCodeExporter commented 9 years ago
AdTsai, to verify the symbolic information you would need to use a symbolic file
reader. Since the formats used by .net and mono are different it doesn't make 
sense
to try to test such functionality. Sorry for that.

Original comment by kump...@gmail.com on 8 Jan 2008 at 10:05

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Ah, right.

With that in mind, I've done the LocalBuilder tests. There were originally 4 
tests, 
but I ran into an odd problem with one of them (it plain crashed the .NET 
runtime) 
so I omitted it. That was tested under Windows, with the Microsoft libraries 
and .NET 2.0, by the way.

Original comment by AdT...@gmail.com on 8 Jan 2008 at 4:50

Attachments:

GoogleCodeExporter commented 9 years ago
Here are the tests for the parameter builder.

The SignatureHelper class already has tests under 
mcs/class/corlib/Test/System.Reflection.Emit/, should I redo them, or just skip 
it?

Original comment by AdT...@gmail.com on 10 Jan 2008 at 6:45

Attachments:

GoogleCodeExporter commented 9 years ago
AdTsai,

The tests for SignatureHelper are too shallow. None of then try to use 
AddArgument,
AddArguments or AddSentinel. Neither test GetSignature and the result byte[].

Testing the encoded signature is a lot of work because you need to learn how 
they are
encoded. You need to read from the ecma 335* standard Partition II chapter 
23.2. It's
quite a lot to diggest, so I'll leave up to you to test the encoded values.

*download it from: 
http://www.ecma-international.org/publications/standards/Ecma-335.htm

Original comment by kump...@gmail.com on 10 Jan 2008 at 1:58

GoogleCodeExporter commented 9 years ago
Hmm, ok. It seems to be quite a complex topic, I'll try to see what I can do. 
But it 
may be a while until I can produce some working tests - it's a lot to read 
through 
and understand.

Original comment by AdT...@gmail.com on 11 Jan 2008 at 6:00

GoogleCodeExporter commented 9 years ago
I've been reading through the official standards document (ECMA-335), and to be 
honest, I don't understand a word of it.

So far, I only have a vague idea of what a signature is, let alone how to 
construct/deconstruct one, how to validate it, and how to check to see if it 
works.

I could probably spend days on this and still not accomplish much - I just 
don't 
think I have enough background knowledge about the technical specifications of 
the 
CLI.

Not sure where I can go from here.

Original comment by AdT...@gmail.com on 12 Jan 2008 at 1:40

GoogleCodeExporter commented 9 years ago
AdTsai, thanks for your contributions. I need to finish reviewing your changes 
this
week. But unfortunelly they won't get commited until 16-jan as I'm really in 
trouble
with internet conectivity.

The next step is exactly to gather information about what areas need atention. 
I'll
open other issues to track these.

Original comment by kump...@gmail.com on 14 Jan 2008 at 4:27

GoogleCodeExporter commented 9 years ago
AdTsai, you could use monocov as described in 
http://www.mono-project.com/Code_Coverage
to see what parts of the other classes in System.Reflection.Emit lacks good 
unit testing.

Original comment by kump...@gmail.com on 14 Jan 2008 at 4:35

GoogleCodeExporter commented 9 years ago
Sorry, but I cannot continue with work on this task.

With school approaching, I no longer have the time to work on this. I'd like to 
un-
claim this task and reopen it up for anyone else wanting to have a shot at it.

Original comment by AdT...@gmail.com on 20 Jan 2008 at 1:26