ParseSplitPaneIntoArgs unit test is failing

[j4james]

Environment

Windows build number: Version 10.0.18362.535
Windows Terminal version (if applicable): Commit 55b6388

Steps to reproduce

1. Build the OpenConsole solution.
2. Run the unit tests with the runut.cmd script.

Expected behavior

All the tests should pass.

Actual behavior

The CommandlineTest::ParseSplitPaneIntoArgs test fails on line 593 with the message:

Error: Verify: AreEqual(SplitState::Vertical, myArgs.SplitStyle()) - Values (1, 4294967295) [File: \terminal\src\cascadia\LocalTests_TerminalApp\CommandlineTest.cpp, Function: TerminalAppLocalTests::CommandlineTest::ParseSplitPaneIntoArgs, Line: 593]
EndGroup: TerminalAppLocalTests::CommandlineTest::ParseSplitPaneIntoArgs [Failed]

That particular error looks to me like a mistake in the test itself. If the command line doesn't include a specific style, I would expect the style to be SplitState::Automatic. I'm guessing this is the result of a change in behavior as the code evolved. And I believe the tests on line 653 and line 707 will fail for the same reason.

Even with those cases fixed, though, there is still a failure which I think is an indication of a real bug in the code. The test on line 633 is failing because it's expecting the style to be Vertical but the return value is Automatic. The code that is to blame is in the AppCommandlineArgs::_buildSplitPaneParser method:

terminal/src/cascadia/TerminalApp/AppCommandlineArgs.cpp

Lines 220 to 230 in b6ec670

	if ((*_horizontalOption || *_verticalOption) && (_splitHorizontal))
	{
	if (_splitHorizontal)
	{
	args->SplitStyle(SplitState::Horizontal);
	}
	else if (_splitVertical)
	{
	args->SplitStyle(SplitState::Horizontal);
	}
	}

There are two problems with this code. The first is that the initial branch isn't even passed unless _splitHorizontal is true, so the _splitVertical case is never handled. The second is that even with that fixed, the _splitVertical case will end up setting the style to Horizontal rather than Vertical.

[DHowett-MSFT]

Yanking triage- i hit this as well, thanks for filing a followup

[DHowett-MSFT]

I've put a test bug into v1.0 because it's in a feature we added and are shipping, and we darn well better make sure it works right.

[zadjii-msft]

Hey I just ran this locally and it looks like we must have fixed this recently. Hard to say for sure, but I bet it was back when we made auto the default behavior.

Fortunately with #6992 set up, we'll be more diligent about these tests in the future. Thanks!