Skip to content

Fix the TabTests, April 2022 edition#12997

Merged
4 commits merged intomainfrom
dev/migrie/b/12158-tab-tests-april-2022
May 5, 2022
Merged

Fix the TabTests, April 2022 edition#12997
4 commits merged intomainfrom
dev/migrie/b/12158-tab-tests-april-2022

Conversation

@zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Apr 28, 2022

In classic fashion, we never run the LocalTests locally before committing, so stuff breaks from time to time.

This time, the main trick was that the tests had a pretty hardcore dependency on the inner workings of _PreviewActionHandler, and when that changed, they broke.

Also, there was a weird crash I saw when I had the default terminal set to the Dev build version. That crash would let the test contents pass, but ultimately fail when TAEF tore down the conhost. Unsetting that fixed the crash 🤷

Closes #12158

@ghost ghost added Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels Apr 28, 2022
@ghost
Copy link

ghost commented Apr 28, 2022

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ zadjii-msft sign now
You have signed the CLA already but the status is still pending? Let us recheck it.

@ItzLevvie ItzLevvie mentioned this pull request Apr 29, 2022
Merged
5 tasks
lhecker
lhecker reviewed May 2, 2022
View reviewed changes
carlos-zamora
carlos-zamora reviewed May 3, 2022
View reviewed changes
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

I'll let Leonard handle giving you the second sign off. I'm really just here to ask if we should try and run the LocalTests as a part of our bug bashes? We should do it on every commit (or at least the relevant ones), but maybe a half-step like this would help? Plus, we want these to go into the helix tests right?

@zadjii-msft
Copy link
Member Author

zadjii-msft commented May 5, 2022

Plus, we want these to go into the helix tests right?

They actually already are! we still just don't get emails from Helix, or something?

@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label May 5, 2022
@ghost
Copy link

ghost commented May 5, 2022

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

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 (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit cd86c0f into main May 5, 2022
@ghost ghost deleted the dev/migrie/b/12158-tab-tests-april-2022 branch May 5, 2022 13:42
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@carlos-zamora carlos-zamora carlos-zamora left review comments

@DHowett DHowett DHowett approved these changes

@lhecker lhecker lhecker approved these changes

+1 more reviewer

@raghavthind2005 raghavthind2005 raghavthind2005 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@lhecker lhecker

Labels

AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TerminalAppLocalTests::TabTests are failing

5 participants

@zadjii-msft @DHowett @lhecker @carlos-zamora @raghavthind2005