LearningTypeScript / projects

Hands-on real world projects that will help you exercise your knowledge of TypeScript.
https://learningtypescript.com/projects
240 stars 160 forks source link

🐛 Bug: projects/arrays/text-processor/src/solution-alternate.ts has some bugs #271

Closed wdpm closed 1 year ago

wdpm commented 1 year ago

Bug Report Checklist

Expected

I forked this test file projects/arrays/text-processor/src/index.test.ts to a new file named projects/arrays/text-processor/src/index-alternate.test.ts and changes a little in order to test projects/arrays/text-processor/src/solution-alternate.ts.

Here is the content of index-alternate.test.ts:

import { describe, expect, test } from "@jest/globals";
import { expectType } from "tsd";

import * as index from "./index";
import * as solution_alternate from "./solution-alternate";

// only for quick testing
process.env.TEST_SOLUTIONS = "1"

const { alignTexts } = process.env.TEST_SOLUTIONS ? solution_alternate : index;

describe(alignTexts, () => {
    describe("types", () => {
        test("function type", () => {
            expectType<
                (
                    texts: string[],
                    options: { align?: "left" | "middle" | "right"; width: number }
                ) => string[][]
            >(alignTexts);
        });

        test("return type", () => {
            expectType<string[][]>(alignTexts([], { width: 0 }));
        });
    });

    const cases: [string[], index.AlignmentOptions, string[][]][] = [
        [[""], { width: 0 }, [[""]]],
        [["", ""], { width: 0 }, [[""], [""]]],
        [[""], { width: 2 }, [["  "]]],
        [["abc"], { width: 5 }, [["abc  "]]],
        [["abc def"], { width: 8 }, [["abc def "]]],
        [
            ["ab de", "abc def"],
            { width: 4 },
            [
                ["ab  ", "de  "],
                ["abc ", "def "],
            ],
        ],
        [
            ["ab de", "abc def", "abcd ef"],
            { width: 4 },
            [
                ["ab  ", "de  "],
                ["abc ", "def "],
                ["abcd", "ef  "],
            ],
        ],
        [["ab de", "abc def"], { width: 5 }, [["ab de"], ["abc  ", "def  "]]],
        [["abc def", "abc def"], { width: 8 }, [["abc def "], ["abc def "]]],
        [
            ["abc def", "abc def ghi"],
            { width: 3 },
            [
                ["abc", "def"],
                ["abc", "def", "ghi"],
            ],
        ],
        [
            ["abc def", "abc def ghi"],
            { width: 5 },
            [
                ["abc  ", "def  "],
                ["abc  ", "def  ", "ghi  "],
            ],
        ],
        [["abc def", "abcdefghi"], { width: 8 }, [["abc def "], ["abcdefghi"]]],
        [[""], { align: "left", width: 0 }, [[""]]],
        [[""], { align: "left", width: 1 }, [[" "]]],
        [[""], { align: "left", width: 2 }, [["  "]]],
        [["abc"], { align: "left", width: 3 }, [["abc"]]],
        [["abc"], { align: "left", width: 4 }, [["abc "]]],
        [["abc"], { align: "left", width: 5 }, [["abc  "]]],
        [["abc def"], { align: "left", width: 7 }, [["abc def"]]],
        [["abc def"], { align: "left", width: 8 }, [["abc def "]]],
        [["abc def"], { align: "left", width: 9 }, [["abc def  "]]],
        [["abc def"], { align: "left", width: 10 }, [["abc def   "]]],
        [["abc def"], { align: "left", width: 11 }, [["abc def    "]]],
        [
            ["a", "ab", "abc", "abcd"],
            { align: "middle", width: 4 },
            [[" a  "], [" ab "], ["abc "], ["abcd"]],
        ],
        [[""], { align: "middle", width: 0 }, [[""]]],
        [[""], { align: "middle", width: 1 }, [[" "]]],
        [[""], { align: "middle", width: 2 }, [["  "]]],
        [["abc"], { align: "middle", width: 3 }, [["abc"]]],
        [["abc"], { align: "middle", width: 4 }, [["abc "]]],
        [["abc"], { align: "middle", width: 5 }, [[" abc "]]],
        [["abc def"], { align: "middle", width: 7 }, [["abc def"]]],
        [["abc def"], { align: "middle", width: 8 }, [["abc def "]]],
        [["abc def"], { align: "middle", width: 9 }, [[" abc def "]]],
        [["abc def"], { align: "middle", width: 10 }, [[" abc def  "]]],
        [["abc def"], { align: "middle", width: 11 }, [["  abc def  "]]],
        [
            ["abc def", "abc def ghi"],
            { align: "middle", width: 5 },
            [
                [" abc ", " def "],
                [" abc ", " def ", " ghi "],
            ],
        ],
        [[""], { align: "right", width: 0 }, [[""]]],
        [[""], { align: "right", width: 1 }, [[" "]]],
        [[""], { align: "right", width: 2 }, [["  "]]],
        [["abc"], { align: "right", width: 3 }, [["abc"]]],
        [["abc"], { align: "right", width: 4 }, [[" abc"]]],
        [["abc"], { align: "right", width: 5 }, [["  abc"]]],
        [["abc def"], { align: "right", width: 7 }, [["abc def"]]],
        [["abc def"], { align: "right", width: 8 }, [[" abc def"]]],
        [["abc def"], { align: "right", width: 9 }, [["  abc def"]]],
        [["abc def"], { align: "right", width: 10 }, [["   abc def"]]],
        [["abc def"], { align: "right", width: 11 }, [["    abc def"]]],
        [
            ["abc def", "abc def ghi"],
            { align: "right", width: 5 },
            [
                ["  abc", "  def"],
                ["  abc", "  def", "  ghi"],
            ],
        ],
    ];

    test.each(cases)("%j %o", (lines, options, aligned) => {
        expect(alignTexts(lines, options)).toEqual(aligned);
    });
});

As usual, I expect all tests will be passed.

Actual

But I got many tests failed.

Here is the key part of the test output:

> test:alertnate
> cross-env TEST_SOLUTIONS=1 jest ./src/index-alternate.test.ts

 FAIL  src/index-alternate.test.ts
  alignTexts
    √ [""] { width: 0 } (2 ms)
    √ ["",""] { width: 0 }
    × [""] { width: 2 } (4 ms)
    × ["abc"] { width: 5 } (1 ms)
    × ["abc def"] { width: 8 } (1 ms)
    × ["ab de","abc def"] { width: 4 } (1 ms)
    × ["ab de","abc def","abcd ef"] { width: 4 } (2 ms)
    × ["ab de","abc def"] { width: 5 }
    × ["abc def","abc def"] { width: 8 } (1 ms)
    √ ["abc def","abc def ghi"] { width: 3 }
    × ["abc def","abc def ghi"] { width: 5 } (1 ms)
    × ["abc def","abcdefghi"] { width: 8 } (1 ms)
    √ [""] { align: 'left', width: 0 }
    × [""] { align: 'left', width: 1 }
    × [""] { align: 'left', width: 2 } (1 ms)
    √ ["abc"] { align: 'left', width: 3 } (1 ms)
    × ["abc"] { align: 'left', width: 4 } (1 ms)
    × ["abc"] { align: 'left', width: 5 } (1 ms)
    √ ["abc def"] { align: 'left', width: 7 }
    × ["abc def"] { align: 'left', width: 8 } (1 ms)
    × ["abc def"] { align: 'left', width: 9 } (1 ms)
    × ["abc def"] { align: 'left', width: 10 }
    × ["abc def"] { align: 'left', width: 11 } (1 ms)
    × ["a","ab","abc","abcd"] { align: 'middle', width: 4 } (1 ms)
    √ [""] { align: 'middle', width: 0 }
    × [""] { align: 'middle', width: 1 } (1 ms)
    × [""] { align: 'middle', width: 2 }
    × ["abc"] { align: 'middle', width: 3 }
    × ["abc"] { align: 'middle', width: 4 } (1 ms)
    √ ["abc"] { align: 'middle', width: 5 }
    × ["abc def"] { align: 'middle', width: 7 }
    × ["abc def"] { align: 'middle', width: 8 } (1 ms)
    √ ["abc def"] { align: 'middle', width: 9 } (1 ms)
    × ["abc def"] { align: 'middle', width: 10 } (1 ms)
    × ["abc def"] { align: 'middle', width: 11 }
    √ ["abc def","abc def ghi"] { align: 'middle', width: 5 } (1 ms)
    √ [""] { align: 'right', width: 0 } (1 ms)
    × [""] { align: 'right', width: 1 }
    × [""] { align: 'right', width: 2 } (1 ms)
    √ ["abc"] { align: 'right', width: 3 }
    × ["abc"] { align: 'right', width: 4 }
    × ["abc"] { align: 'right', width: 5 } (1 ms)
    √ ["abc def"] { align: 'right', width: 7 }
    × ["abc def"] { align: 'right', width: 8 } (1 ms)
    × ["abc def"] { align: 'right', width: 9 }
    × ["abc def"] { align: 'right', width: 10 } (1 ms)
    × ["abc def"] { align: 'right', width: 11 } (1 ms)
    × ["abc def","abc def ghi"] { align: 'right', width: 5 }
    types
      √ function type (1 ms)
      √ return type

Impacted Project

projects/arrays/text-processor/src/solution-alternate.ts

Additional Info

So I read through this file solution-alternate.ts and found some issues.

const aligners = {
    left: (line: string, width: number) => line.padEnd(width),
    middle: (line: string, width: number) => {
        const remainingSpaces = width - line.length;

        return remainingSpaces
            ? [
                // #3 use string.repeat method is more succinct, The result of Array.from() is weird on some test cases.
                // Array.from({ length: Math.floor(remainingSpaces / 2) }, () => " "),
                " ".repeat(Math.floor(remainingSpaces / 2)),
                line,
                // Array.from({ length: Math.ceil(remainingSpaces / 2) }, () => " ")
                " ".repeat(Math.ceil(remainingSpaces / 2))
            ].join("")
            : line;
    },
    right: (line: string, width: number) => line.padStart(width) // #1 should be padStart, not padEnd
};

function alignLines(
    lines: string[],
    { align = "left", width }: AlignmentOptions
) {
    const aligner = aligners[align];
    // #2 original logic is wrong
    return lines.map((line, index, lines) => {
        return aligner(line, width);
    });
}

Reproduce Array.from() bug

// Problematic case:
// [["abc def"], { align: "middle", width: 11 }, [["  abc def  "]]],

const remainingSpaces = 4
let s = [
    // #3
    Array.from({ length: Math.floor(remainingSpaces / 2) }, () => " "),
    "abc def",
    Array.from({ length: Math.ceil(remainingSpaces / 2) }, () => " ")
].join("");

console.log(s)
// , abc def ,
// Note the result has two comma `,` marks, not got "  abc def  ".

final

After the fix above, all tests have passed in my local enviroment.

JoshuaKGoldberg commented 1 year ago

👍 thanks for pointing this out! Solution files are generally checked in CI, but because this is an alternate one it's not being verified.

Would you be open to sending a PR?

wdpm commented 1 year ago

@JoshuaKGoldberg refer to #272.