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

Add testing infrastructure for illinker related changes #37258

Closed
marek-safar opened this issue Jun 1, 2020 · 9 comments · Fixed by #37618
Closed

Add testing infrastructure for illinker related changes #37258

marek-safar opened this issue Jun 1, 2020 · 9 comments · Fixed by #37618
Assignees
Labels
area-Infrastructure linkable-framework Issues associated with delivering a linker friendly framework

Comments

@marek-safar
Copy link
Contributor

There is no testing infrastructure and no tests available to verify changes which require add/remove/change annotation or XML descriptors for illinker to work with src/libraries. As we keep adding and modifying them it becomes harder to review such PRs with such changes.

We should come with the testing apps (no testing framework!) which each will verify the code which is supposed to keep the dynamically accessed members.

@vitek-karas @danmosemsft

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-Host untriaged New issue has not been triaged by the area owner labels Jun 1, 2020
@ghost
Copy link

ghost commented Jun 1, 2020

Tagging subscribers to this area: @vitek-karas, @swaroop-sridhar
Notify danmosemsft if you want to be subscribed.

@marek-safar marek-safar added area-Infrastructure and removed area-Host untriaged New issue has not been triaged by the area owner labels Jun 1, 2020
@ghost
Copy link

ghost commented Jun 1, 2020

Tagging subscribers to this area: @ViktorHofer
Notify danmosemsft if you want to be subscribed.

@vitek-karas
Copy link
Member

@eerhardt

@marek-safar marek-safar changed the title Add testing infrastructure for trimming changes Add testing infrastructure for illinker related changes Jun 1, 2020
@eerhardt
Copy link
Member

eerhardt commented Jun 1, 2020

@joperezr was starting to look at #943. Is this a duplicate of that issue? Or is #943 strictly for measuring size, and this is strictly for measuring correctness?

@eerhardt eerhardt added the linkable-framework Issues associated with delivering a linker friendly framework label Jun 1, 2020
@eerhardt eerhardt added this to Committed in Linker optimization work Jun 1, 2020
@marek-safar
Copy link
Contributor Author

This is not about size at all but about correctness. For example, I should get test failure for every DynamicDependency I remove from the codebase, same with every entry in linker descriptor

@joperezr
Copy link
Member

joperezr commented Jun 1, 2020

That was kind of what I thought #943 was tracking but apparently that seems to be size-specific then. This is more or less the plan we have so far on how to add testing for this feature work:

As @eerhardt outlined in his design doc(https://github.com/eerhardt/designs/blob/AddLinkingLibrariesDoc/accepted/2020/linking-libraries.md#testing) there are two major things we will want to add testing for:

  1. Does the application still work?
  2. Is the application smaller?

For the first one, we can use this issue to track it. The plan we have so far is to add simple console applications that would try to use the API in question, link them, and ensuring that they can still run by ensuring the application returned some previously defined success return code. (so as to try not to depend on anything else)
We are still checking how to approach this as it will be hard to scale with thousands of simple apps checked in to the runtime repo for this, so I will update this issue once we have an actual plan.

For the second one, we plan on having some tests in the performance repo where we take full Blazor cannonical applications (as well as default template apps) and linking them to establish a baseline, and then running those in some cadence to ensure we don't regress size on disk.

@marek-safar
Copy link
Contributor Author

Yes, this is only about "Does the application still work" like scenarios, the other testing scenarios should be discussed in other issues.

Couple of things to consider for implementation.

  • One for all solution might not work here
  • The apps will need to run with all illinker sensitive RIDs, most of them are supported in self-contained scenarios only.
  • Be able to run the application would be nice but it might be hard in practice as that makes the isolation much harder

@joperezr
Copy link
Member

joperezr commented Jun 2, 2020

One for all solution might not work here

Right, the idea would be to have one simple console app consuming very limited set of the library's API which just a) uses the API causing the trimmable code to be called and then b) returns a special return code like 100 for example. Every single console app like this would be considered one test, and we would want to have several tests like these per assembly as if we only had one per assembly it is likely that most things would be kept due to code coverage being larger.

The apps will need to run with all illinker sensitive RIDs, most of them are supported in self-contained scenarios only.

We have to think more about this, to know if we will want to run these tests on build machine or on Helix runs instead. If on build machines, then the advantage would be that we already have verticals for most of these RIDs so we would have the coverage that we need. For now, I believe we would only be running with self-contained in mind.

Be able to run the application would be nice but it might be hard in practice as that makes the isolation much harder

Not sure I fully understand this point. The idea would be to produce the console apps as I outlined above, and then just running them and checking the return code is what we expected (100), if not then we would assume that something that should have been kept was trimmed.

@marek-safar
Copy link
Contributor Author

Not sure I fully understand this point.

The point was that in some cases to run the app which includes specific API you want to check you might need to do extra work (add more code to run successfully) and that will alter linker analysis so you will be testing different setup.

Secondly, how do you plan to run cross-compiled tests? Are you going to setup e.g. Android test environment during the build if you plan to run the apps?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure linkable-framework Issues associated with delivering a linker friendly framework
Projects
Development

Successfully merging a pull request may close this issue.

5 participants