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

Unit tests to illustrate issue #39 #40

Merged
merged 3 commits into from
Sep 28, 2020

Conversation

ProductiveRage
Copy link
Contributor

@ProductiveRage ProductiveRage commented Apr 13, 2020

Tests created to try to explain myself better in relation to issue #39 (and to offer some regression tests as well as failure cases).

…oblem and two pass already and only exist as regression tests
… a property instead of a string[] that is being directly deserialised to; again two tests that fail to illustrate the problem and two that pass to act as regression tests

When the code is updated to fix the problem in the previous changeset, I'm sure that it will fix these as well but I thought that it made sense to include a few extra cases
@RicoSuter
Copy link
Owner

I've fixed the tests - I commented one out as I think there is no way to support that (the information is just not available via reflection). What do you think?

@ProductiveRage
Copy link
Contributor Author

I think that it's a mistake to comment out the test because it's a valid use case. Unfortunately, you are correct that the information does not appear to be available in meta data via the normal forms of reflection but it is in there - I have just struggled so far to get it out!

It might be interesting to know that the project PublicApiGenerator can make the distinction but it uses Mono.Cecil to get the information out. I know that your project here requires zero dependencies, which is very attractive, but it should be feasible to look into the the Mono.Cecil library is doing and utilise the same logic. I started looking at this a few weeks ago but I got distracted by other things.

The good news, then, is that this information is available - the bad news is that it's not yet clear how to get it out (though hopefully Cecil's source code can help with that!)

@RicoSuter
Copy link
Owner

Probably in PublicApiGenerator (as well as the NJS/NSwag schema generators) always reflect via property or method (return) parameter where you have this info. Here we only have the runtime type here. Maybe we can get the metadata via generic type on the method...

@ProductiveRage
Copy link
Contributor Author

ProductiveRage commented May 26, 2020 via email

@RicoSuter RicoSuter merged commit 1991689 into RicoSuter:master Sep 28, 2020
@RicoSuter
Copy link
Owner

I merged this for now, the missing test can be fixed later... but as i said i dont know whether this is even possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants