[browser][icu][libraries] Load full ICU in library tests only when it's necessary #82619
Conversation
ilonatommy
added
arch-wasm
| @@ -35,6 +32,11 @@ | |||
| <_BundleAOTTestWasmAppForHelixDependsOn>$(_BundleAOTTestWasmAppForHelixDependsOn);PrepareForWasmBuildApp;_PrepareForAOTOnHelix</_BundleAOTTestWasmAppForHelixDependsOn> | |||
| </PropertyGroup> | |||
|
|
|||
| <!-- trimming tests InvariantGlobalizationFalse require full ICU data --> | |||
| <PropertyGroup Condition="'$(TestTrimming)' == 'true'"> | |||
There was a problem hiding this comment.
Why only when 'TestTrimming' ?
There was a problem hiding this comment.
That's the story:
I made this PR to reduce the number of tests that run with this flag set to true to minimum. However, I was not able to make InvariantGlobalizationFalse.cs in trimming tests pass - even setting the Property straight in that test d1f019b was not woring - when we were entering WasmApp.targets the value was already trimmed.
I came up with the workaround: I won't set the flag for all the tests in tests.browser.targets but only for the trimming ones, to make InvariantGlobalizationFalse.cs pass.
There was a problem hiding this comment.
You also need '$(IsTrimmingTestProject)' == 'true'. $(TestTrimming) would be set for the whole build, but it is relevant only for trimming projects which will have $(IsTrimmingTestProject) set. At least based on reading the code:)
There was a problem hiding this comment.
Changing it to IsTrimmingTestProject breaks the test again. It has to be connected with the order of MsBuild files evaluation, because for the discussed test both flags: IsTrimmingTestProject and TestTrimming are true in the binlog. So it must be that we are reading test.browser.targets before we read Directory.Build.props where IsTrimmingTestProject value is set. TestTrimming is set in src/libraries/tests.proj and is already availabe in the line quoted.
Conclusion: the condition has to stay as it is.
There was a problem hiding this comment.
I'm confused about it. Do all combinations 4 of trimming and invariant work for native rebuild ?
Is this just issue with unit testing ?
There was a problem hiding this comment.
We took it offline with Pavel and it's not about trimming but about these files being used in different builds. That is why values set in one of them are not reflected in the other.
There was a problem hiding this comment.
There is nested MSBuild which didn't receive WasmIncludeFullIcuData from System.Runtime.TrimmingTests.proj
And then full file is not there!
|
/azp run runtime-wasm-libtests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Failures unrelated. |
|
/azp run runtime-wasm |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ghost
locked as resolved and limited conversation to collaborators
Fixes #82592.