Skip to content

Commit a42d072

Browse files
BridgeARMylesBorins
authored andcommitted
assert: use object argument in innerFail
Right now it is difficult to know what argument stands for what property. By refactoring the arguments into a object it is clear what stands for what. Backport-PR-URL: #23223 PR-URL: #17582 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
1 parent 84948cf commit a42d072

4 files changed

Lines changed: 109 additions & 30 deletions

File tree

‎lib/assert.js‎

Lines changed: 97 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -38,22 +38,26 @@ const assert = module.exports = ok;
3838
// both the actual and expected values to the assertion error for
3939
// display purposes.
4040

41-
functioninnerFail(actual,expected,message,operator,stackStartFunction){
42-
thrownewerrors.AssertionError({
43-
message,
44-
actual,
45-
expected,
46-
operator,
47-
stackStartFunction
48-
});
41+
functioninnerFail(obj){
42+
thrownewerrors.AssertionError(obj);
4943
}
5044

51-
functionfail(actual,expected,message,operator,stackStartFunction){
52-
if(arguments.length===1)
45+
functionfail(actual,expected,message,operator,stackStartFn){
46+
constargsLen=arguments.length;
47+
48+
if(argsLen===1){
5349
message=actual;
54-
if(arguments.length===2)
50+
}elseif(argsLen===2){
5551
operator='!=';
56-
innerFail(actual,expected,message,operator,stackStartFunction||fail);
52+
}
53+
54+
innerFail({
55+
actual,
56+
expected,
57+
message,
58+
operator,
59+
stackStartFn: stackStartFn||fail
60+
});
5761
}
5862
assert.fail=fail;
5963

@@ -67,37 +71,71 @@ assert.AssertionError = errors.AssertionError;
6771
// Pure assertion tests whether a value is truthy, as determined
6872
// by !!value.
6973
functionok(value,message){
70-
if(!value)innerFail(value,true,message,'==',ok);
74+
if(!value){
75+
innerFail({
76+
actual: value,
77+
expected: true,
78+
message,
79+
operator: '==',
80+
stackStartFn: ok
81+
});
82+
}
7183
}
7284
assert.ok=ok;
7385

7486
// The equality assertion tests shallow, coercive equality with ==.
7587
/* eslint-disable no-restricted-properties */
7688
assert.equal=functionequal(actual,expected,message){
7789
// eslint-disable-next-line eqeqeq
78-
if(actual!=expected)innerFail(actual,expected,message,'==',equal);
90+
if(actual!=expected){
91+
innerFail({
92+
actual,
93+
expected,
94+
message,
95+
operator: '==',
96+
stackStartFn: equal
97+
});
98+
}
7999
};
80100

81101
// The non-equality assertion tests for whether two objects are not
82102
// equal with !=.
83103
assert.notEqual=functionnotEqual(actual,expected,message){
84104
// eslint-disable-next-line eqeqeq
85105
if(actual==expected){
86-
innerFail(actual,expected,message,'!=',notEqual);
106+
innerFail({
107+
actual,
108+
expected,
109+
message,
110+
operator: '!=',
111+
stackStartFn: notEqual
112+
});
87113
}
88114
};
89115

90116
// The equivalence assertion tests a deep equality relation.
91117
assert.deepEqual=functiondeepEqual(actual,expected,message){
92118
if(!innerDeepEqual(actual,expected,false)){
93-
innerFail(actual,expected,message,'deepEqual',deepEqual);
119+
innerFail({
120+
actual,
121+
expected,
122+
message,
123+
operator: 'deepEqual',
124+
stackStartFn: deepEqual
125+
});
94126
}
95127
};
96128
/* eslint-enable */
97129

98130
assert.deepStrictEqual=functiondeepStrictEqual(actual,expected,message){
99131
if(!innerDeepEqual(actual,expected,true)){
100-
innerFail(actual,expected,message,'deepStrictEqual',deepStrictEqual);
132+
innerFail({
133+
actual,
134+
expected,
135+
message,
136+
operator: 'deepStrictEqual',
137+
stackStartFn: deepStrictEqual
138+
});
101139
}
102140
};
103141

@@ -572,30 +610,53 @@ function objEquiv(a, b, strict, keys, memos) {
572610
// The non-equivalence assertion tests for any deep inequality.
573611
assert.notDeepEqual=functionnotDeepEqual(actual,expected,message){
574612
if(innerDeepEqual(actual,expected,false)){
575-
innerFail(actual,expected,message,'notDeepEqual',notDeepEqual);
613+
innerFail({
614+
actual,
615+
expected,
616+
message,
617+
operator: 'notDeepEqual',
618+
stackStartFn: notDeepEqual
619+
});
576620
}
577621
};
578622

579623
assert.notDeepStrictEqual=notDeepStrictEqual;
580624
functionnotDeepStrictEqual(actual,expected,message){
581625
if(innerDeepEqual(actual,expected,true)){
582-
innerFail(actual,expected,message,'notDeepStrictEqual',
583-
notDeepStrictEqual);
626+
innerFail({
627+
actual,
628+
expected,
629+
message,
630+
operator: 'notDeepStrictEqual',
631+
stackStartFn: notDeepStrictEqual
632+
});
584633
}
585634
}
586635

587636
// The strict equality assertion tests strict equality, as determined by ===.
588637
assert.strictEqual=functionstrictEqual(actual,expected,message){
589638
if(actual!==expected){
590-
innerFail(actual,expected,message,'===',strictEqual);
639+
innerFail({
640+
actual,
641+
expected,
642+
message,
643+
operator: '===',
644+
stackStartFn: strictEqual
645+
});
591646
}
592647
};
593648

594649
// The strict non-equality assertion tests for strict inequality, as
595650
// determined by !==.
596651
assert.notStrictEqual=functionnotStrictEqual(actual,expected,message){
597652
if(actual===expected){
598-
innerFail(actual,expected,message,'!==',notStrictEqual);
653+
innerFail({
654+
actual,
655+
expected,
656+
message,
657+
operator: '!==',
658+
stackStartFn: notStrictEqual
659+
});
599660
}
600661
};
601662

@@ -643,18 +704,27 @@ function innerThrows(shouldThrow, block, expected, message) {
643704
details+=` (${expected.name})`;
644705
}
645706
details+=message ? `: ${message}` : '.';
646-
fail(actual,expected,`Missing expected exception${details}`,'throws');
707+
innerFail({
708+
actual,
709+
expected,
710+
operator: 'throws',
711+
message: `Missing expected exception${details}`,
712+
stackStartFn: innerThrows
713+
});
647714
}
648715
if(expected&&expectedException(actual,expected)===false){
649716
throwactual;
650717
}
651718
}elseif(actual!==undefined){
652719
if(!expected||expectedException(actual,expected)){
653720
details=message ? `: ${message}` : '.';
654-
fail(actual,
655-
expected,
656-
`Got unwanted exception${details}`,
657-
'doesNotThrow');
721+
innerFail({
722+
actual,
723+
expected,
724+
operator: 'doesNotThrow',
725+
message: `Got unwanted exception${details}`,
726+
stackStartFn: innerThrows
727+
});
658728
}
659729
throwactual;
660730
}

‎lib/internal/errors.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ class AssertionError extends Error {
8080
if(typeofoptions!=='object'||options===null){
8181
thrownewexports.TypeError('ERR_INVALID_ARG_TYPE','options','object');
8282
}
83-
var{ actual, expected, message, operator,stackStartFunction}=options;
83+
var{ actual, expected, message, operator,stackStartFn}=options;
8484
if(message){
8585
super(message);
8686
}else{
@@ -99,7 +99,7 @@ class AssertionError extends Error {
9999
this.actual=actual;
100100
this.expected=expected;
101101
this.operator=operator;
102-
Error.captureStackTrace(this,stackStartFunction);
102+
Error.captureStackTrace(this,stackStartFn);
103103
}
104104
}
105105

‎test/message/error_exit.out‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Exiting with code=1
22
assert.js:*
3-
throw new errors.AssertionError({
3+
throw new errors.AssertionError(obj);
44
^
55

66
AssertionError [ERR_ASSERTION]: 1 === 2

‎test/parallel/test-assert.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -721,3 +721,12 @@ common.expectsError(
721721
/* eslint-enable no-restricted-properties */
722722
assert(7);
723723
}
724+
725+
common.expectsError(
726+
()=>assert.ok(null),
727+
{
728+
code: 'ERR_ASSERTION',
729+
type: assert.AssertionError,
730+
message: 'null == true'
731+
}
732+
);

0 commit comments

Comments
 (0)