test: add `--trace-gc` flag test by tony-go · Pull Request #42471 · nodejs/node · GitHub

Hi 👋

Context

This pull request follows this previous one and the initiative started by the diagnostic wg.

Added

Now that --trace-gc is a part of the diagnostic tooling support, I thought that we could improve maturity tiers by adding a simple high-level test. As --trace-gc is a part of the v8 codebase, it probably doesn't make sense to test specific cases here. I just tried to test the integration between node and v8.

I first tried to find where this integration was made, but I didn't find it. While I read the code I expected to find it somewhere around here:

node/src/node_options.cc

Line 238 in de163d5

StringVector* const v8_args,

If someone has an idea about where the integration is made, let me know ^^

How to run this test? -> make test-v8-updates

I wasn't sure about where should the test be or how I should write it, so I try to make it simple. 🙈

cc @nodejs/diagnostics

LGTM.

LGTM with or without the suggested nits fixed.

CI: https://ci.nodejs.org/job/node-test-pull-request/43207/

Co-authored-by: Luigi Pinca <luigipinca@gmail.com>

Suggested change

      
                const expectedRegex = new RegExp(expectedOutput[index]);
          
                assert.match(line, expectedRegex);
          
                assert.match(line, expectedOutput[index]);

expectedOutput already contains regexes.

In fact, I did like it first, then I got an error but I misread it.

I finally trim the output string and it worked ^^

Thanks a lot 🙏

CI: https://ci.nodejs.org/job/node-test-pull-request/43216/

Landed in 00cb091

PR-URL: #42471 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>

PR-URL: nodejs#42471 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>

PR-URL: #42471 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>

PR-URL: nodejs#42471 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>

PR-URL: #42471 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>

PR-URL: nodejs/node#42471 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>

test: add trace-gc flag test

1442662

nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Mar 25, 2022

jasnell reviewed Mar 25, 2022

View reviewed changes

Comment thread test/fixtures/gc.js Outdated

jasnell approved these changes Mar 25, 2022

View reviewed changes

test: add line break in gc fixture

edccf35

RafaelGSS approved these changes Mar 26, 2022

View reviewed changes

gireeshpunathil approved these changes Mar 26, 2022

View reviewed changes

RaisinTen approved these changes Mar 26, 2022

View reviewed changes

This comment was marked as outdated.

Sign in to view

github-actions Bot mentioned this pull request Mar 27, 2022

CI Reliability 2022-03-27 nodejs/reliability#233

Open

26 tasks

lpinca approved these changes Mar 27, 2022

View reviewed changes

Comment thread test/v8-updates/test-trace-gc-flag.js Outdated

Comment thread test/v8-updates/test-trace-gc-flag.js Outdated

Flarna approved these changes Mar 27, 2022

View reviewed changes

github-actions Bot mentioned this pull request Mar 28, 2022

CI Reliability 2022-03-28 nodejs/reliability#234

Open

24 tasks

RaisinTen added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Mar 28, 2022

Tony Gorez and others added 2 commits March 28, 2022 08:55

doc: nitpick fix

85d8e3b

Co-authored-by: Luigi Pinca <luigipinca@gmail.com>

test: use assert.match instead of assert.ok

7442e90

targos reviewed Mar 28, 2022

View reviewed changes

test: delete Regex call and trim string output

2f7fb1a

tony-go requested a review from targos March 28, 2022 10:26

Flarna approved these changes Mar 28, 2022

View reviewed changes

Flarna added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 28, 2022

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 28, 2022

Flarna added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Mar 28, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 28, 2022

nodejs-github-bot merged commit 00cb091 into nodejs:master Mar 28, 2022

This was referenced Apr 5, 2022

V17.8.1 proposal #42612

Closed

v17.9.0 proposal #42613

Merged

tony-go mentioned this pull request Apr 6, 2022

Re-evaluate --trace-gc in the tools tier-list nodejs/diagnostics#554

Closed

tony-go mentioned this pull request May 5, 2022

doc: add diagnosis memory leak guide nodejs/nodejs.org#4590

Closed

juanarbol mentioned this pull request May 31, 2022

2022-05-XX, Version 16.16.0 'Gallium' (LTS) #43268

Closed

targos mentioned this pull request Aug 3, 2022

v16.17.0 release proposal #44098

Merged

test: add `--trace-gc` flag test by tony-go · Pull Request #42471 · nodejs/node · GitHub

Uh oh!

Uh oh!

test: add --trace-gc flag test - #42471

test: add --trace-gc flag test#42471
nodejs-github-bot merged 5 commits into
nodejs:masterfrom
tony-go:test_trace_gc_integration

tony-go commented Mar 25, 2022 •
edited
Loading

Uh oh!

Uh oh!

Uh oh!

tony-go commented Mar 26, 2022

Uh oh!

RafaelGSS left a comment

Uh oh!

This comment was marked as outdated.

lpinca left a comment

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Mar 28, 2022

Uh oh!

targos Mar 28, 2022

Uh oh!

tony-go Mar 28, 2022

Uh oh!

nodejs-github-bot commented Mar 28, 2022

Uh oh!

nodejs-github-bot commented Mar 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

		const expectedRegex = new RegExp(expectedOutput[index]);
		assert.match(line, expectedRegex);

	const expectedRegex = new RegExp(expectedOutput[index]);
	assert.match(line, expectedRegex);
	assert.match(line, expectedOutput[index]);

test: add `--trace-gc` flag test by tony-go · Pull Request #42471 · nodejs/node · GitHub

Uh oh!

Uh oh!

test: add --trace-gc flag test - #42471

test: add --trace-gc flag test#42471nodejs-github-bot merged 5 commits intonodejs:masternodejs/node:masterfrom tony-go:test_trace_gc_integrationtony-go/node:test_trace_gc_integrationCopy head branch name to clipboard

Conversation

tony-go commented Mar 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Added

Uh oh!

Uh oh!

tony-go commented Mar 26, 2022

Uh oh!

RafaelGSS left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

lpinca left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Mar 28, 2022

Uh oh!

targos Mar 28, 2022

Choose a reason for hiding this comment

Uh oh!

tony-go Mar 28, 2022

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Mar 28, 2022

Uh oh!

nodejs-github-bot commented Mar 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

test: add --trace-gc flag test#42471
nodejs-github-bot merged 5 commits into
nodejs:masterfrom
tony-go:test_trace_gc_integration

tony-go commented Mar 25, 2022 •
edited
Loading