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

Libraries trimming test are missing a lot of configurations #39274

Open
marek-safar opened this issue Jul 14, 2020 · 28 comments
Open

Libraries trimming test are missing a lot of configurations #39274

marek-safar opened this issue Jul 14, 2020 · 28 comments
Assignees
Labels
area-Infrastructure linkable-framework Issues associated with delivering a linker friendly framework os-ios Apple iOS test-enhancement Improvements of test source code
Milestone

Comments

@marek-safar
Copy link
Contributor

marek-safar commented Jul 14, 2020

Newly added trimming libraries tests run only for configurations which are not by default trimmed aggressively. All the ones which are and are used to customers are missing so we have no test coverage.

Mandatory RIDs to be added to the existing matrix

  • browser-wasm
  • ios-arm64
  • tvos-arm64
  • android-arm64
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jul 14, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Jul 14, 2020

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

@safern
Copy link
Member

safern commented Jul 14, 2020

Cc: @joperezr

@joperezr
Copy link
Member

Although we do plan to enable a few extra platforms, with the trimming tests we are manually changing the way we invoke the linker in order to force aggressive trimming so we do have coverage for now. That said, I do want to restate that we do plan to add at least wasm platform as well

@safern safern removed the untriaged New issue has not been triaged by the area owner label Jul 14, 2020
@safern
Copy link
Member

safern commented Jul 14, 2020

@joperezr could you add the right milestone on when you're going to be adding these? At least browser seems important for 5.0.0

@safern safern closed this as completed Jul 14, 2020
@safern safern reopened this Jul 14, 2020
@joperezr joperezr added this to the 5.0.0 milestone Jul 14, 2020
@joperezr joperezr added the test-enhancement Improvements of test source code label Jul 14, 2020
@joperezr
Copy link
Member

yup, wasm will be done before 5.0 definitely.

@ViktorHofer
Copy link
Member

@joperezr it's unclear to me what the remaining work is. Is someone actively working on this? Thanks

@joperezr
Copy link
Member

We still haven't added a wasm leg for the linker tests, today we only have these 3:

platforms:
- Windows_NT_x64
- OSX_x64
- Linux_x64

Ideally we would want to add wasm in there as well to build that vertical and run linker tests for that runtime, so that would be the remaining work. @eerhardt do we still want to do that for 5.0?

@eerhardt
Copy link
Member

Ideally we would want to add wasm in there as well to build that vertical and run linker tests for that runtime, so that would be the remaining work. @eerhardt do we still want to do that for 5.0?

It would be good to get the wasm leg in, but I don't think it is required for 5.0 at this point. It can be done early in 6.0.

@danmoseley
Copy link
Member

Added to story #43078

@marek-safar
Copy link
Contributor Author

I think this is ready from our side. Running the linker against listed RIDs should be possible a long time ago and we now have AndroidAppBuilderTask, AppleAppBuilderTask and WasmBuildApp to produce the executable and Helix integration to run it.

@danmoseley
Copy link
Member

@eerhardt could you summarize the current thinking here -- I believe you talked to @marek-safar -- what are the right next actions here do you see?

@eerhardt
Copy link
Member

We will start off by adding a wasm leg, and then ios and android legs to the trimming tests.

@danmoseley
Copy link
Member

sounds good. To summarize my understanding,

  • we don't need Xamarin/Blazor libraries/templates/SDK support here; that kind of testing has to happen upstack, both functionally, and size (latter in perf repo)
  • we need to publish for their RIDs as that swaps in different implementations of our libraries in some cases
  • the tests verify that (a) there are no linker errors/warnings (b) libraries functionality doesn't break under trimming
  • we write these tests to focus on particular "tricky" API we have annotated -- eg., where there is serialization - to ensure our annotations remain good

@eerhardt
Copy link
Member

That summary looks good except:

the tests verify that (a) there are no linker errors/warnings (b) libraries functionality doesn't break under trimming

Technically (a) happens during the build, and not during the tests. The tests verify "(b) libraries functionality doesn't break under trimming".

@danmoseley
Copy link
Member

Ah, saw warningsaserrors on the test project build and assumed it was for that purpose. Fine!

@eerhardt
Copy link
Member

Moving to 7.0. We added a browser-wasm leg, which covers the basic mono scenarios. We should enable android and ios in 7.0.

@eerhardt eerhardt modified the milestones: 6.0.0, 7.0.0 Jul 30, 2021
@steveisok
Copy link
Member

This is something my team can help out on. These tests seem similar to our functional tests and I don't think would take too much effort to enable.

@carlossanlop
Copy link
Member

@joperezr @steveisok should we move this to 8.0?

@joperezr
Copy link
Member

joperezr commented Jul 6, 2022

I'll defer to @steveisok on this. Ideally we would want to have these tests running on all different runtimes that we cross-target, but the most important thing here is to decide when do we want to do that work.

@ViktorHofer
Copy link
Member

ping @steveisok for the above question

@steveisok
Copy link
Member

Let's leave it in 7. I could end up being wrong, but I believe the work is hooking up to our mobile testing targets the same way the functional tests (src/tests/FunctionalTests) do. If I'm correct, then I think it's less than a few days of work at worst.

@ericstj
Copy link
Member

ericstj commented Aug 16, 2022

@steveisok - should we keep this in 7.0 or move it out at this point?

@ericstj ericstj added the arch-wasm WebAssembly architecture label Aug 16, 2022
@ghost
Copy link

ghost commented Aug 16, 2022

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Newly added trimming libraries tests run only for configurations which are not by default trimmed aggressively. All the ones which are and are used to customers are missing so we have no test coverage.

Mandatory RIDs to be added to the existing matrix

  • browser-wasm
  • ios-arm64
  • tvos-arm64
  • android-arm64
Author: marek-safar
Assignees: -
Labels:

arch-wasm, area-Infrastructure-libraries, test-enhancement, linkable-framework

Milestone: 7.0.0

@lewing lewing modified the milestones: 7.0.0, 8.0.0 Aug 17, 2022
@directhex
Copy link
Member

Are we good on this for wasm, i.e. was #48429 everything needed? If so, I can duplicate that PR's changes for iOS, tvOS and Android

@steveisok
Copy link
Member

Are we good on this for wasm, i.e. was #48429 everything needed? If so, I can duplicate that PR's changes for iOS, tvOS and Android

As far as I know. The wiring up of these tests for mobile should be the same as the functional tests.

@lewing lewing added os-ios Apple iOS and removed arch-wasm WebAssembly architecture labels Jul 11, 2023
@ghost
Copy link

ghost commented Jul 11, 2023

Tagging subscribers to 'os-ios': @steveisok, @akoeplinger, @kotlarmilos
See info in area-owners.md if you want to be subscribed.

Issue Details

Newly added trimming libraries tests run only for configurations which are not by default trimmed aggressively. All the ones which are and are used to customers are missing so we have no test coverage.

Mandatory RIDs to be added to the existing matrix

  • browser-wasm
  • ios-arm64
  • tvos-arm64
  • android-arm64
Author: marek-safar
Assignees: directhex
Labels:

arch-wasm, area-Infrastructure-libraries, test-enhancement, linkable-framework, os-ios

Milestone: 8.0.0

@ghost
Copy link

ghost commented Jul 12, 2023

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

Newly added trimming libraries tests run only for configurations which are not by default trimmed aggressively. All the ones which are and are used to customers are missing so we have no test coverage.

Mandatory RIDs to be added to the existing matrix

  • browser-wasm
  • ios-arm64
  • tvos-arm64
  • android-arm64
Author: marek-safar
Assignees: directhex
Labels:

test-enhancement, area-Infrastructure, linkable-framework, os-ios

Milestone: 8.0.0

@steveisok steveisok modified the milestones: 8.0.0, Future Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Infrastructure linkable-framework Issues associated with delivering a linker friendly framework os-ios Apple iOS test-enhancement Improvements of test source code
Projects
Status: No status
Development

No branches or pull requests