Fix the TabTests! #3833
Merged
Fix the TabTests! #3833
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(cherry picked from commit b392ba0)
This helper doesn't solve our problems, but it's there.
1. Make TerminalAppLib _not_ build App.xaml. I moved it to
TerminalApp.vcxproj, so only the DLL builds it. No idea if that actually
builds the DLL, package correctly, I'm just trying to get the XAML test to
work.
2. This causes weird bugs trying to import the TerminalAppLib project, for
whatever reason. Unfortunately, it causes the TerminalApp types to not
appear in the final manifest, so I think we'll need to reference that winmd
manually.
3. Make sure to include the `("UAP:Host", "Xaml")` bit in the test metadata.
4. Now, the `RunOnUIThread` test actually passes. It won't work with our
types, but _progress_
I need to revert the App.xaml->TerminalApp change, because that's going to totally mess up the build. Full stop, that needs to happen. The problem we're running into now is that _ConsoleGenerateAdditionalWinmdManifests doesn't generate a .manifest for TerminalAppLib, so it never gets included into the Appxmanifest. I could try manually adding the TerminalApp types to the manifest prototype again. That's an option. But since I'll need to revert the App change anyways, idk if that'll matter. Maybe if we manually _don't_ include App.xbf in the output, and TerminalApp.App in the manifest?
This reverts commit 80e9e73.
This reverts commit 51bc3e7.
Just don't include the App.xbf and it magic works? Somewhere after we create the TermControl we crash, but that's _so_ good man. We're so close.
|
(draft PRs don't run the CI but treat this as a draft) |
They didn't work, and won't till we get an updated TAEF
zadjii-msft
requested review from
DHowett-MSFT,
carlos-zamora,
miniksa and
leonMSFT
zadjii-msft
added
Area-Build
|
(this guy is ready for review now) |
src/cascadia/LocalTests_TerminalApp/TerminalApp.LocalTests.vcxproj
Outdated
Show resolved
Hide resolved
carlos-zamora
approved these changes
Dec 6, 2019
|
I love these commit messages. |
miniksa
reviewed
Dec 6, 2019
src/cascadia/LocalTests_TerminalApp/TerminalApp.LocalTests.AppxManifest.prototype.xml
Show resolved
Hide resolved
src/cascadia/LocalTests_TerminalApp/TerminalApp.LocalTests.vcxproj
Outdated
Show resolved
Hide resolved
src/cascadia/LocalTests_TerminalApp/TerminalApp.LocalTests.vcxproj
Outdated
Show resolved
Hide resolved
|
@miniksa @carlos-zamora @DHowett-MSFT @leonMSFT Heads up, if you want to run the LocalTests after merging this - you'll need to manually delete the old |
DHowett-MSFT
reviewed
Dec 6, 2019
| @@ -77,7 +76,7 @@ | |||
| <DisableSpecificWarnings>4702;%(DisableSpecificWarnings)</DisableSpecificWarnings> | |||
| </ClCompile> | |||
| <Link> | |||
| <AdditionalDependencies>User32.lib;WindowsApp.lib;shell32.lib;%(AdditionalDependencies)</AdditionalDependencies> | |||
| <AdditionalDependencies>onecoreuap.lib;%(AdditionalDependencies)</AdditionalDependencies> | |||
There was a problem hiding this comment.
I mean, wait, if this is using the cppwinrt pre props you don't even need the link block at all.
There was a problem hiding this comment.
The cppwinrt preprops only was WindowsApp.lib...
DHowett-MSFT
approved these changes
Dec 6, 2019
zadjii-msft
added
the
AutoMerge
|
Hello @zadjii-msft! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
deleted the
dev/migrie/b/fix-tab-tests-harder-better-faster-stronger
branch
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of the Pull Request
Fix the
TabTests, and enable testing of types with XAML content. TheTabTestswere written many, many moons ago. they were intended to be our tests of XAML-like content within the Terminal app, so we could have unittests of Tabs, Panes, etc. Between their initial authoring and the day they were checked in, we had a bunch of build changes come in and break them irreperably.We've gotten them fixed now with one weird trick doctors hate me. As long as there isn't an
App.xbfin the test's output directory, then the tests will deploy just fine.We also needed a bit of magic, cribbed straight from TAEF, to enable running test code synchronously on the UI thread. Hence,
CppwinrtTailored.h.References
PR Checklist
Validation Steps Performed