Add some tests for TextBuffer::Reflow #8715

DHowett · 2021-01-06T20:04:06Z

This is by no means comprehensive. It will be unmarked as draft when it
is more comprehensive.

This pull request adds some tests for resizing a TextBuffer and
reflowing its contents. Each test takes the form of an initial state and
a number of buffers of different sizes. The initial state is used to
seed the first TextBuffer, and the subsequent buffers are only used to
compare.

I manually reimplemented some of the DBCS logic to ensure that the
buffers contain exactly what they're supposed to. I know this is
non-ideal. After some of the CharRow changes in #8446 land, this will
need to be updated.

There's a cool bit of TAEF gore in here: the IDataSource. An IDataSource
allows us to programmatically return test cases. It's a code-only
version of its support for parameterized tests of the form Data:x = {0, 1, 2} .

The only downsides are...

It looks like COM (it is not using COM under the hood, just the COM
ABI)
Property values must be returned as strings.

To best support rich test types, I used IDataSource to produce a lit of
array indices and nothing more. The test is run once for array member,
and it is the test's responsibility to look up the object to which that
index refers.

Works great though! Each reflow test is its own unit, and a failure in
an earlier reflow test will not tank a later one.

DHowett · 2021-01-06T20:06:36Z

Looks like this:

StartGroup: ReflowTests::TestReflowCases#export0
TAEF: Data[index]: 0
[0.0] Test case "SBCS, cursor remains in buffer, no circling, no original wrap"
[0.1] Resizing to 5x5
[0.2] Resizing to 6x5
[0.3] Resizing to 7x5
EndGroup: ReflowTests::TestReflowCases#export0 [Passed]

StartGroup: ReflowTests::TestReflowCases#export1
TAEF: Data[index]: 1
[1.0] Test case "SBCS, cursor remains in buffer, no circling, with original wrap"
[1.1] Resizing to 5x5
[1.2] Resizing to 6x5
[1.3] Resizing to 7x5
EndGroup: ReflowTests::TestReflowCases#export1 [Passed]

StartGroup: ReflowTests::TestReflowCases#export2
TAEF: Data[index]: 2
[2.0] Test case "DBCS, cursor remains in buffer, no circling, with original wrap"
[2.1] Resizing to 5x5
[2.2] Resizing to 6x5
[2.3] Resizing to 7x5
[2.4] Resizing to 8x5
EndGroup: ReflowTests::TestReflowCases#export2 [Passed]

we can customize that name (export0) if we want.

DHowett · 2021-01-11T22:26:09Z

There are 30 conditional branches in Reflow.

I hit 22 of them.
3 are error cases (never hit).
5 are in viewport position retention (the std::optional arguments).

22 + 3 + 5 = 30

# Conflicts: # .github/actions/spell-check/expect/expect.txt

DHowett · 2021-01-12T22:31:39Z

/azp run

azure-pipelines · 2021-01-12T22:31:55Z

Azure Pipelines successfully started running 1 pipeline(s).

carlos-zamora · 2021-01-13T00:37:38Z

src/buffer/out/ut_textbuffer/ReflowTests.cpp

+                    { 5, 5 }, // reduce width by 1
+                    {
+                        { L"ABCDE", true },
+                        { L"F$   ", false }, // [BUG] EXACT WRAP. $ should be alone on next line.


Should these [BUG] tags be filed and linked appropriately?

I'm probably not gonna file 20 new bugs for "reflow is broken", which is a well-known statement.

I should have used a less controversial word, like "[KNOWN BADNESS]"

carlos-zamora · 2021-01-13T00:39:08Z

src/buffer/out/ut_textbuffer/ReflowTests.cpp

+    };
+
+#pragma region TAEF hookup for the test case array above
+    struct ArrayIndexTaefAdapterRow : public Microsoft::WRL::RuntimeClass<Microsoft::WRL::RuntimeClassFlags<Microsoft::WRL::ClassicCom | Microsoft::WRL::InhibitFtmBase>, IDataRow>


It'd be cool if this got pulled out so that other tests could take advantage of this. Alas, that's already on your radar and probably out of scope for this PR.

ghost · 2021-01-18T21:51:19Z

Hello @DHowett!

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.

This is by no means comprehensive. It will be unmarked as draft when it is more comprehensive. This pull request adds some tests for resizing a TextBuffer and reflowing its contents. Each test takes the form of an initial state and a number of buffers of different sizes. The initial state is used to seed the first TextBuffer, and the subsequent buffers are only used to compare. I manually reimplemented some of the DBCS logic to ensure that the buffers contain _exactly_ what they're supposed to. I know this is non-ideal. After some of the CharRow changes in microsoft#8446 land, this will need to be updated. There's a cool bit of TAEF gore in here: the IDataSource. An IDataSource allows us to programmatically return test cases. It's a code-only version of its support for parameterized tests of the form `Data:x = {0, 1, 2}` . The only downsides are... 1. It looks like COM (it is not using COM under the hood, just the COM ABI) 2. Property values must be returned as strings. To best support rich test types, I used IDataSource to produce _a lit of array indices_ and nothing more. The test is run once for array member, and it is the test's responsibility to look up the object to which that index refers. Works great though! Each reflow test is its own unit, and a failure in an earlier reflow test will not tank a later one.

Add Reflow tests

2362721

DHowett marked this pull request as draft January 6, 2021 20:04

DHowett added 3 commits January 6, 2021 13:01

More cases!

c376ea7

Even more test cases

3a38f69

Eh, just allow-list the lot of em

fe7cac2

DHowett marked this pull request as ready for review January 11, 2021 22:26

DHowett added 2 commits January 11, 2021 14:27

Merge remote-tracking branch 'origin/main' into dev/duhowett/reflow

afee0fc

# Conflicts: # .github/actions/spell-check/expect/expect.txt

sources

5f1758a

DHowett mentioned this pull request Jan 12, 2021

[Epic] Text Buffer rewrite #8000

Closed

15 tasks

carlos-zamora approved these changes Jan 13, 2021

View reviewed changes

zadjii-msft assigned miniksa Jan 15, 2021

miniksa approved these changes Jan 15, 2021

View reviewed changes

DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 18, 2021

ghost merged commit fa0cd8c into main Jan 18, 2021

ghost deleted the dev/duhowett/reflow branch January 18, 2021 21:51

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add some tests for TextBuffer::Reflow #8715

Add some tests for TextBuffer::Reflow #8715

DHowett commented Jan 6, 2021

DHowett commented Jan 6, 2021

DHowett commented Jan 11, 2021

DHowett commented Jan 12, 2021

azure-pipelines bot commented Jan 12, 2021

carlos-zamora Jan 13, 2021

DHowett Jan 13, 2021

carlos-zamora Jan 13, 2021

ghost commented Jan 18, 2021

Add some tests for TextBuffer::Reflow #8715

Add some tests for TextBuffer::Reflow #8715

Conversation

DHowett commented Jan 6, 2021

DHowett commented Jan 6, 2021

DHowett commented Jan 11, 2021

DHowett commented Jan 12, 2021

azure-pipelines bot commented Jan 12, 2021

carlos-zamora Jan 13, 2021

Choose a reason for hiding this comment

DHowett Jan 13, 2021

Choose a reason for hiding this comment

carlos-zamora Jan 13, 2021

Choose a reason for hiding this comment

ghost commented Jan 18, 2021

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.

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.