★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/21985/filesNouvelle recherche | Portail wanayoo
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: remove unused config #21985

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+6 −15
Diff settings

Always

Just for now

@@ -5,8 +5,6 @@ const bench = common.createBenchmark(main, {
n: [12e6]
});

process.maxTickDepth = Infinity;

function main({ n }) {
let counter = n;
function cb4(arg1, arg2, arg3, arg4) {
@@ -4,8 +4,6 @@ const bench = common.createBenchmark(main, {
n: [12e6]
});

process.maxTickDepth = Infinity;

function main({ n }) {
let counter = n;
bench.start();
@@ -23,12 +23,8 @@
require('../common');
const assert = require('assert');

// this is the inverse of test-next-tick-starvation.

This comment has been minimized.

Copy link
@refack

refack Jul 26, 2018

Member

Could we keep the test description (these 3 lines)

This comment has been minimized.

Copy link
@benjamingr

benjamingr Jul 26, 2018

Author Member

Sure, can revert this - though I didn't find it very informative if you do I'll revert.

This comment has been minimized.

Copy link
@benjamingr

benjamingr Jul 26, 2018

Author Member

I'll revert except the third line

// it verifies that process.nextTick will *always* come before other
// events, up to the limit of the process.maxTickDepth value.

// WARNING: unsafe!
process.maxTickDepth = Infinity;
// this is the inverse of test-next-tick-starvation. it verifies
// that process.nextTick will *always* come before other events

let ran = false;
let starved = false;
@@ -22,13 +22,13 @@
'use strict';
const common = require('../common');
const Readable = require('stream').Readable;

// This tests synchronous read callbacks and verifies that even if they nest
// heavily the process handles it without an error

const r = new Readable();
const N = 256 * 1024;

// Go ahead and allow the pathological case for this test.

This comment has been minimized.

Copy link
@refack

This comment has been minimized.

Copy link
@benjamingr

benjamingr Jul 26, 2018

Author Member

It no longer allows the pathological use case though since maxTickDepth was removed so the comment is incorrect (and has been since 0.12). Is there alternative phrasing you'd prefer?

This comment has been minimized.

Copy link
@refack

refack Jul 26, 2018

Member

I did not associate the comment specifically with maxTickDepth. If you are sure it's related, then I would assume the whole test is obsolete, and should be removed.

This comment has been minimized.

Copy link
@benjamingr

benjamingr Jul 26, 2018

Author Member

@refack the test verifies the behaviour of a synchronous read on the stream. I've added a comment I think makes the most sense. I think benjamingr@782149d explains why it was added

// Yes, it's an infinite loop, that's the point.
process.maxTickDepth = N + 2;

let reads = 0;
r._read = function(n) {
const chunk = reads++ === N ? null : Buffer.allocUnsafe(1);
@@ -39,7 +39,6 @@ if (process.argv[2] !== 'child') {

const domain = require('domain');
const d = domain.create();
process.maxTickDepth = 10;

// in the error handler, we trigger several MakeCallback events
d.on('error', function() {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.