개요
이번 PR은 #57418에서 시작된 이슈를 보고 시작했다.
https://github.com/nodejs/node/issues/57418
-cpu-prof-name 'CPU.${pid}.cpuprofile'옵션을 줘도,${pid}가 치환되지 않고 문자 그대로 출력되는 문제가 있었다.- 공식 문서에도
-cpu-prof-name옵션은 "입력한 값 그대로 사용"한다고 명확히 적혀 있었지만, 클러스터 환경 등에서 프로세스별로 프로파일 파일명이 겹치는 문제를 해결하기 위해 placeholder 치환 기능을 요청하는 의견이 많았다.
향후 이와 같은 placeholder 치환 기능 추가를 검토할 가치가 있다고 판단하여 기능 추가를 해보았다.
이전 기여에선 문서 기여라서 테스트 / 빌드 / CI 테스트 통과 이런 건 별로 신경 안 썼는데 막힌 부분들도 같이 정리해보았다.
목적
기존의 --cpu-prof-name 옵션은 CPU 프로파일 파일 이름을 고정된 문자열로만 지정할 수 있었는데, 여기에 ${pid}를 넣으면 현재 프로세스 PID가 자동으로 치환되도록 개선한 것이다. 이를 통해 동시에 여러 프로파일을 생성할 때 파일 충돌을 방지할 수 있다.
변경 사항
src/inspector_profiler.cc에서${pid}문자열을 감지해ReplacePlaceholders()함수를 새로 작성하여 실제 프로세스 ID 값으로 대체하도록 구현.- 해당 기능이 정상 동작하는지 검증하는 테스트 코드도 함께 작성. (
test/sequential/test-cpu-prof-name.js) - 문서에는
${pid}placeholder가 어떻게 동작하는지에 대한 설명을 새로 추가. (doc/api/cli.md)
테스트
기능을 변경하거나 추가했다면 반드시 관련된 테스트를 직접 실행해서 동작을 확인해야 한다.
$ make test-only이전 기여에선 문서만 수정해서 테스트를 추가할 일이 없었지만 이번 PR은 기능이 추가되었기 때문에 리뷰어가 점검할 수 있는 테스트 코드를 추가해달라고 했다.
!! 테스트도 파일이 여러개 였고 기능 별로 나뉘어져 있었다.
테스트 종류 요약
| 디렉터리 | 설명 |
|---|---|
parallel/ | 병렬 실행 가능 (가장 일반적) |
sequential/ | 순차 실행 필수. 테스트 간 영향 우려 있을 때 사용 |
cctest/ | C++ native 단위 테스트. 임베더용 |
addons/ | native 확장 모듈 관련 테스트 |
wpt/ | Web Platform Test. 브라우저 호환성 검사용 |
- 예:
test/parallel/test-buffer-readuint.js
테스트 구성 관련 파일
.status: flaky 테스트 조건 지정testcfg.py: 테스트 그룹 정의
git push 하기 전 로컬 사전 점검
Node.js에 기여하면서 점검하면 좋을 체크리스트를 적어봤다. 실수로 인해 CI 실패를 막기 위한 핵심 항목들이다.
아래 사항을 몰랐기에 다시 수정하는 커밋 올리는 일 없게 미리 체크하는 게 좋을 것 같다.
다 몰라서 CI 테스트에서 Fail 떴던 항목들이다.
1. 모든 테스트 돌리기
$ ./configure
$ make -j4
$ make test-only또는 빠르게 테스트하려면:
$ python3 tools/test.py변경한 부분만 골라서 → 필요한 테스트만 골라서 진행하면 좀 더 빨리 끝난다.
make test-only 같은 명령은 시간이 너무 오래 걸리기 때문에 변경한 영역에 영향을 미치는 테스트만 골라서 해도 좋은 것 같다.
$ python3 tools/test.py parallel/test-stream-*2. Lint 검사 (코딩 스타일)
$ make lint→ 위 명령은 모든 린트를 다 검사하고 수정해준다.
$ make lint-js
$ make lint-cpp→ 만일 본인이 js / cpp 만 수정했다면 위 명령으로 각각 파일들만 수정해준다.
3. 포매팅 검사
- C++:
make format-cpp - Markdown:
make format-md
CI에서 린트, 포맷팅 문제로 fail 날 수 있으므로 PR 올리기 전에 꼭 확인하자.
→ js는 자동 포맷팅은 없고 린트만 통과하면 된다. (doc/STYLE_GUIDE.md 에 스타일 가이드는 있다고 함.)
4. 커밋 메시지 규칙
Node.js는 semantic commit 스타일을 따른다:
<subsystem>: <short description>예:
cli: support ${pid} placeholder in --cpu-prof-name- subsystem 예시:
stream,fs,http,doc,cli등 - 메시지 길이는 72자를 넘기지 말 것. 넘기면 CI 실패함.. → 추가로
feat는 CI 테스트 때 걸립니다. 사용하면 안 됨.
추가적으로
CI 다시 트리거 하는 법
만일 본인이 문서만 건드렸거나 이전 커밋까진 CI가 다 통과되었었고 이번에 갑자기 fail 되었다면, flasky 이슈로 안 된 걸 수도 있다.
제가 이전 커밋 때는 CI 성공했었고 이번엔 문서만 수정했는데 실패가 된 적이 있었습니다.
테스트 오류로 잘못된 것 같아서 ci를 다시 돌리고 싶었는데 권한이 없어서 rerun을 못 시킨다고 합니다
이때는 빈 커밋을 추가해 CI를 다시 트리거할 수 있다.
git commit --allow-empty -m "trigger CI rerun"
git push origin <branch-name>--allow-empty는 아무 변경 없는 커밋을 추가하는 옵션
이 커밋이 푸시되면 GitHub가 자동으로 CI를 다시 실행함
Jenkins CI Test
처음 PR 생성 후 수정 커밋들까지는 계속 GitHub Actions에서 CI가 트리거되었다. 그런데 마지막에는 갑자기 Jenkins에서 CI가 돌아가고, 아래 3개 항목에서 테스트 실패(Fail)가 떴다. 처음엔 코드 문제인 줄 알고 당황했지만, 확인해 보니 여기서도 flaky 때문이었다.
node-test-commit — tests failed
node-test-commit-aix — tests failed
node-test-pull-request — tests failedNode.js에서 Jenkins가 트리거되는 경우 (공식문서 doc/contributing/commit-queue)
- PR에
commit-queue또는commit-queue-squash라벨이 붙으면 - Node.js GitHub Bot이 해당 PR을 Commit Queue에 넣고,
- Jenkins에서 전체 플랫폼에 대한 CI를 실행함 (Linux, macOS, Windows, AIX 등 다양한 환경에서 테스트)
즉, PR 머지를 위한 최종 자동화 단계로 Jenkins 기반의 풀 테스트가 실행된 것.
정리
기능 구현, 테스트 작성까지 다뤘던 의미 있는 경험이었다. Node.js는 구조가 탄탄한 프로젝트인 만큼 규칙도 엄격하지만, 그만큼 배우는 게 많다.
이번 기여에서 계속 리뷰어들한테 일주일동안 무시당하다가 정대연 멘토님이 승인해주셨다. 감사합니다
PR 올려놓고 조급해하기보다는 그냥 마음 편하게 먹고 기다려야 될 것 같다.
OSSCA 참여하길 잘한 것 같다. 말년이라 비교적으로 부대에서, 휴가나와서도 시간이 많다 보니 재밌다.
그리고 오픈소스에 기여할 때는 정말 기여하고 싶다고 마음을 먹지 않는 이상 하기 힘들 것 같다.
본인이 하고 싶다고 시간을 오래 한 번만 투자한다고 되는 게 아니라 리뷰어들이 피드백을 해줄 때마다 PR 확인하고 수정해나가는 과정이 필요하기 때문이다.
