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.

Sign up for GitHub

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

Jump to bottom

specify Win10 + maxversiontested to enable xaml APIs to be used in tests running under testhost.exe #4888

Merged
merged 5 commits into from
Feb 26, 2024
Merged

specify Win10 + maxversiontested to enable xaml APIs to be used in tests running under testhost.exe #4888

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cbf0a5c
specify Win10 + maxversiontested to enable xaml APIs to be used in tests
ChrisGuzak Feb 7, 2024
e0659c6
remove redundant Win10, move maxversiontested under 10
ChrisGuzak Feb 7, 2024
448e173
go with a single maxversion tested, fixe quotes
ChrisGuzak Feb 19, 2024
e0fcc53
use 18226 per the quirk def
ChrisGuzak Feb 19, 2024
7a8e0a1
use first release OS version after the quirk test
Feb 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/testhost.x86/app.manifest
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<!-- Adding same as used in TPv1 Refer: /src/vset/Agile/TestPlatform/RocksteadyCLI/app.manifest -->
<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
<!-- The assemblyIdentity node is needed to fix a binding problem on Win2003 (PS72518) -->
Expand All @@ -7,6 +7,9 @@
<application>
<!-- Windows 10 -->
<supportedOS Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}"/>
<maxversiontested Id='10.0.18362.0'/>
<maxversiontested Id='10.0.19041.0'/>
<maxversiontested Id='10.0.22000.0'/>
<!-- Windows 8.1 -->
<supportedOS Id="{1f676c76-80e1-4239-95bb-83d0f6d0da78}"/>
<!-- Windows Vista -->
Expand All @@ -24,4 +27,4 @@
</requestedPrivileges>
</security>
</trustInfo>
</assembly>
</assembly>
7 changes: 5 additions & 2 deletions src/testhost/app.manifest
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<!-- Adding same as used in TPv1 Refer: /src/vset/Agile/TestPlatform/RocksteadyCLI/app.manifest -->
<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
<!-- The assemblyIdentity node is needed to fix a binding problem on Win2003 (PS72518) -->
Expand All @@ -7,6 +7,9 @@
<application>
<!-- Windows 10 -->
<supportedOS Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}"/>
<maxversiontested Id='10.0.18362.0'/>
<maxversiontested Id='10.0.19041.0'/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two questions:

  1. Have you confirmed that this doesn't prevent tests from running on RS5 (10.0.17763)?
  2. Should newer versions be added as well? Like 22621 (22H2) and 22631 (23H2)?

Copy link
Member Author

@ChrisGuzak ChrisGuzak Feb 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MSLukeWest I've not tested this on older versions, but I understand this will have no effect on them. and newer versions should not be needed... @asklar can you help me explain why? I recall you (or Gov?) recommended multiple to support running on intermediate versions. I read this bug but don't understand its implications or if it relates to this quirk or others. In your sample you specify one value 10.0.18362.0 (the next highest released build relative to 10.0.18226.0?). In my testing 10.0.18226.0 works on testhost.exe.

          <QUIRK NAME="BlockXamlIslands" CODE_ID="262" ENABLED_VERSION_LT="10.0.18226.0" CONTACT_ALIAS="turekhe" ID="{57064B35-BE7D-4111-A3D0-FF077347F207}">
            <DESCRIPTION>
              Xaml Islands were first shipped in RS5. It was just in preview mode and an incomplete feature.
              However, Islands in 19H1 are more stable and complete. 
              Hence Islands are officially available in 19H1 and onward releases and are blocked on all prior releases.
            </DESCRIPTION>
          </QUIRK>

I'd prefer to go with a single value 10.0.18226.0 to match the quirk, so I've updated this PR to do that.

Let me know what you think (I emailed gov too, does he have a github identity?).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the original bug I had filed is https://task.ms/33687330
IIUC you have to specify 18362 for xaml islands, and there's another value for segoe ui variable support. Adding a later value doesn't enable the features unlocked by an earlier value which is what makes this maxversiontested flag so confusing. @DHowett might remember this more

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

specifying 18226 works in my testing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MSLukeWest per the email thread I've updated this change to use 10.0.18362.0, the first public release of the os after 10.0.18226.0.

its ready to go so please merge it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, @nohwnd please merge for us

<maxversiontested Id='10.0.22000.0'/>
<!-- Windows 8.1 -->
<supportedOS Id="{1f676c76-80e1-4239-95bb-83d0f6d0da78}"/>
<!-- Windows Vista -->
Expand All @@ -24,4 +27,4 @@
</requestedPrivileges>
</security>
</trustInfo>
</assembly>
</assembly>
5 changes: 4 additions & 1 deletion src/vstest.console/app.manifest
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
<application>
<!-- Windows 10 -->
<supportedOS Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}"/>
<maxversiontested Id='10.0.18362.0'/>
<maxversiontested Id='10.0.19041.0'/>
<maxversiontested Id='10.0.22000.0'/>
<!-- Windows 8.1 -->
<supportedOS Id="{1f676c76-80e1-4239-95bb-83d0f6d0da78}"/>
<!-- Windows Vista -->
Expand All @@ -24,4 +27,4 @@
</requestedPrivileges>
</security>
</trustInfo>
</assembly>
</assembly>
Loading