# 7. [Microsoft / playwright] feat(matcher): add new GenericAssertions(toBeOneOf)

pengooseDev·2024년 12월 6일
0
post-thumbnail

1. Issue

issue #24232

토의가 활발히 일어났던 이슈.

GenericAssertions에 두 가지 메서드를 추가해달라는 요청이다.
해당 기능이 제공된다면, 조금 더 간단하고 직관적으로 여러 상황에 따른 case들을 테스트에서 핸들링할 수 있다.

두 개의 메서드를 전부 구현해 PR을 무겁게 생성하기보단,
toBeOneOf를 먼저 추상화 한 후 PR > 리뷰 > merged > follow-PR 방식이 구현 및 리뷰자 관점에서 효율적이라 판단했다.


2. Context 파악

코드 보기

matchers.ts에 여러 GenericAssertions들이 외부 모듈(jest)에 의존성을 강하게 가진 상태로 제공되고 있는 상태이다.

// ex) MatchersObject.toBe()
const matchers: MatchersObject = {
  toBe(received: unknown, expected: unknown) {
    const matcherName = 'toBe';
    const options: MatcherHintOptions = {
      comment: 'Object.is equality',
      isNot: this.isNot,
      promise: this.promise,
    };

    const pass = Object.is(received, expected);

    const message = pass
      ? () =>

        matcherHint(matcherName, undefined, undefined, options) +
        '\n\n' +
        `Expected: not ${printExpected(expected)}`
      : () => {
        const expectedType = getType(expected);

        let deepEqualityName = null;
        if (expectedType !== 'map' && expectedType !== 'set') {
          // If deep equality passes when referential identity fails,
          // but exclude map and set until review of their equality logic.
          if (
            equals(
                received,
                expected,
                [...this.customTesters, ...toStrictEqualTesters],
                true,
            )
          )
            deepEqualityName = 'toStrictEqual';
          else if (
            equals(received, expected, [
              ...this.customTesters,
              iterableEquality,
            ])
          )
            deepEqualityName = 'toEqual';

        }

        return (

          matcherHint(matcherName, undefined, undefined, options) +
          '\n\n' +
          (deepEqualityName !== null
            ? `${DIM_COLOR(
                `If it should pass with deep equality, replace "${matcherName}" with "${deepEqualityName}"`,
            )}\n\n`
            : '') +
          printDiffOrStringify(
              expected,
              received,
              EXPECTED_LABEL,
              RECEIVED_LABEL,
              isExpand(this.expand),
          )
        );
      };

    // Passing the actual and expected objects so that a custom reporter
    // could access them, for example in order to display a custom visual diff,
    // or create a different error message
    return { actual: received, expected, message, name: matcherName, pass };
  },

어떤 방식으로 API를 제공하는지 파악하고 docs를 읽어 금방 context를 파악할 수 있었다.


3. 회귀 테스트 추가

기존 TC와는 컨벤션이 많이 다른 것을 알 수 있다. 해당 모듈은 애초에 third-party 모듈로 구분되어, 가독성보단 다양한 테스트케이스를 적용해 엣지 케이스를 최소화 하는 것을 목표로 두는 것 같았다.

  • playwright의 자체 API는 테스트 환경 구동에서 꽤 많은 리소스와 시간을 잡아먹는다. 그렇기에 테스트양이 많아질수록 오버헤드가 발생하는 것을 경계하라는 피드백을 받은 적이 있었다.

    다만, third-party 외부 라이브러리 테스트는 로컬에서 자체적인 빌드 후 커밋과 함께 스냅샷을 비교하는 테스트로 진행되고 있었다. (npm run ttest에서 1000번 후반대 테스트 진행 후 갑자기 1000개 가량이 짧은 시간 안에 실행되는 테스트)

가독성이 떨어지는 건 알지만, 테스트 컨벤션이 아래와 같아 비슷하게 적용하였다.

test.describe('.toBeOneOf()', () => {
  const matchingCases = [
    [2, [1, 2, 3]],
    ['b', ['a', 'b', 'c']],
    [true, [false, true]],
    [null, [undefined, null]],
    [undefined, [undefined, null]],
    [NaN, [NaN, 1, 2]],
    [BigInt(1), [BigInt(1), BigInt(2)]],
    [[1, 2], [[1, 2], [3, 4]]],
    [{ a: 1 }, [{ a: 1 }, { b: 2 }]],
    [{ a: { b: { c: 1 } } }, [{ a: { b: { c: 1 } } }, { a: { b: { c: 2 } } }]],
  ];

  const nonMatchingCases = [
    [4, [1, 2, 3]],
    ['d', ['a', 'b', 'c']],
    [false, [true]],
    [null, [undefined]],
    [undefined, [null]],
    [NaN, [1, 2]],
    [BigInt(3), [BigInt(1), BigInt(2)]],
    [[1, 2], [[3, 4], [5, 6]]],
    [{ a: 1 }, [{ b: 2 }, { c: 3 }]],
    [{ a: { b: { c: 1 } } }, [{ a: { b: { c: 2 } } }]],
  ];

  matchingCases.forEach(([value, array]: [any, any]) => {
    test(`passes when the value is in the expected array: expect(${stringify(value)}).toBeOneOf(${stringify(array)})`, () => {
      expectUnderTest(value).toBeOneOf(array);
    });
  });

  nonMatchingCases.forEach(([value, array]: [any, any]) => {
    test(`fails when the value is not in the expected array: expect(${stringify(value)}).toBeOneOf(${stringify(array)})`, () => {
      expect(() => expectUnderTest(value).toBeOneOf(array)).toThrowErrorMatchingSnapshot();
    });
  });

  nonMatchingCases.forEach(([value, array]: [any, any]) => {
    test(`passes when using .not and value is not in the expected array: expect(${stringify(value)}).not.toBeOneOf(${stringify(array)})`, () => {
      expectUnderTest(value).not.toBeOneOf(array);
    });
  });

  matchingCases.forEach(([value, array]: [any, any]) => {
    test(`fails when using .not and value is in the expected array: expect(${stringify(value)}).not.toBeOneOf(${stringify(array)})`, () => {
      expect(() => expectUnderTest(value).not.toBeOneOf(array)).toThrowErrorMatchingSnapshot();
    });
  });

  test('supports asymmetric matchers within the expected array', () => {
    expectUnderTest({ a: 1, b: 2 }).toBeOneOf([
      { a: 1 },
      expect.objectContaining({ b: 2 }),
    ]);
    expectUnderTest('hello world').toBeOneOf([
      expect.stringContaining('world'),
      'hello',
    ]);
  });

  test('fails when value does not match any asymmetric matchers in expected array', () => {
    expect(() =>
      expectUnderTest({ a: 1, b: 2 }).toBeOneOf([
        { a: 2 },
        expect.objectContaining({ c: 3 }),
      ]),
    ).toThrowErrorMatchingSnapshot();
  });

  test('assertion error matcherResult property contains matcher name, expected and actual values', () => {
    const actual = 5;
    const expected = [1, 2, 3];
    try {
      expectUnderTest(actual).toBeOneOf(expected);
    } catch (error) {
      expect(error.matcherResult).toEqual(
          expect.objectContaining({
            actual,
            expected,
            name: 'toBeOneOf',
          }),
      );
    }
  });
});

4. 구현 및 문서작성

  toBeOneOf(received: unknown, expected: Array<unknown>) {
    const matcherName = 'toBeOneOf';
    const isNot = this.isNot;
    const options: MatcherHintOptions = {
      isNot,
      promise: this.promise,
    };

    if (!Array.isArray(expected)) {
      throw new Error(
          matcherErrorMessage(
              matcherHint(matcherName, undefined, undefined, options),
              `${EXPECTED_COLOR('expected')} value must be an array`,
              printWithType('Expected', expected, printExpected),
          ),
      );
    }

    // ⛳️ Nested된 참조데이터용 deepEquals 알고리즘
    const pass = expected.some(item =>
      equals(received, item, [...this.customTesters, iterableEquality]),
    );

    const message = () =>
      matcherHint(matcherName, undefined, undefined, options) +
      '\n\n' +
      `Expected: ${isNot ? 'not ' : ''}to be one of ${printExpected(expected)}\n` +
      `Received: ${printReceived(received)}`;
    
    // ⛳️ 일부 메서드에선 pass, message만 제공하였으나, 확장성 있는 컨벤션 따르기로 함.
    return { actual: received, expected, message, name: matcherName, pass };
  },
## method: GenericAssertions.toBeOneOf
* since: v1.49.1

Ensures that value is deeply equal to one of the elements in the expected array.

**Usage**

'''js
const value = 2;
expect(value).toBeOneOf([1, 2, 3]);
expect(value).not.toBeOneOf([4, 5, 6]);

const obj = { a: 1 };
expect(obj).toBeOneOf([{ a: 1 }, { b: 2 }]);
expect(obj).not.toBeOneOf([{ a: 2 }, { b: 3 }]);
'''

### param: GenericAssertions.toBeOneOf.expected
* since: v1.49.1
- `expected` <[Array]<[any]>>

Expected array to match against.

5. PR 및 Issue Closed

> PR

구현하면서 조금 오래된 Issue이고 GenericAssertions가 1.9v이 업데이트되며, 최초에 제기된 issue를 해결할 수 있는 몇 가지 메서드가 추가되었다. 이를 뒤늦게 인지한 것은 구현이 끝난 뒤 문서작업 컨벤션을 확인하는 과정이었다.

그래도 기능 자체는 다르기 때문에, 굳이 Issue창에서 묻는 것보단 PR을 남겨두고 피드백을 받는게 낫다고 판단하였다.

오픈소스 팀이 워낙 바쁘다보니 오래된 issue가 관리되지 않았던 것이다. 팀 내 토의를 통해, playwright에 해당 API를 지원하지 않으시는 것이 결론났고, PR은 closed되었다.

처음엔 조금 아쉬움이 들었다.(노력했으나 결과가 나오지 않았다고 생각했던 것 같다.)
문득, 감정을 복기하다보니 몇 가지들을 깨닫게 되었고, 오히려 본질적인 기여에 한 걸음 다가갔다는 생각이 들기 시작했다.


6. 기여의 본질?

우리는 종종 형태에 집착한다.
형태에 집착하면, 이는 우리의 눈을 가리고 본질을 보지 못하게 한다.

PR이 closed 되었을 때, 느껴진 아쉬움에서 내가 오픈소스에 가지고 있던 태도를 추론할 수 있었다.

  • docs가 아닌 feat, fix에 대한 기여를 높이 평가하거나, Contribute 후 프로필 창에 기여 표시. 즉, 기여의 형태에 집착하고 있었던 것이다.

물론, 이렇게 진행한 기여 또한 멋진 기여임에는 틀림이 없지만, 필자가 도달하고자 하는 이상적인 오픈소스 활동과는 거리가 있었다.

필자가 이루고자 하는 이상적인 기여란 단순히 내가 작성한 코드의 병합을 의미하지 않았다.

하나의 오픈소스에 꾸준히 활동하며 하나의 팀이 되어가는 것.
PR이나 issue를 통해 피드백 받고, 토의하고, 팀의 컨벤션과 철학을 알아가며, 때로는 현재 PR처럼 팀들이 신경쓰지 못하는 부분을 서포팅 해주는 것이 진정한 하나의 팀. 그리고 기여가 되어가는 과정이라고 생각하기 때문이다.

쉽게 말해 필자가 생각하는 "기여"란 팀에 도움이 되었고 모종의 가치를 창출했다면 그걸로 OK라는 것이다.

즉, playwright 팀이 신경쓰지 못했던 부분을 PR을 통해 팀 토의로 이끌고, issue를 close 하였다는 그 자체가 멋진 기여라고 생각되었다.


나는 처음이지만, maintainer는 아닐껄?

분명, 오픈소스 maintainer들은 이런 상황(PR Close)에 직면한 적이 여러번 존재할 것이다.
PR이 Closed 된다면 그 기여자가 다시 해당 프로젝트에 기여할 확률이 줄어든다는 것을 그들은 경험적으로 알 것이라 생각된다. 거절은 종종 상처로 다가오기 때문이다.

오픈소스 maintainer들은 기여자의 감정적인 부분들까지 신경써줘야 한다는 것을 깨닫고, 그들이 짊어진 무게를 조금 더 알게 되었다.

몇몇 오픈소스에는 기여 과정에서 아쉬웠던 부분들이 종종 존재했으나, 가장 즐거운 기여 경험은 playwright라고 장담할 수 있다. 멋진 팀의 일원이 될 수 있기를 바란다.

0개의 댓글