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

isAtLeast() creates misleading output #1518

Open
UlrichEckhardt opened this issue Apr 28, 2023 · 2 comments
Open

isAtLeast() creates misleading output #1518

UlrichEckhardt opened this issue Apr 28, 2023 · 2 comments

Comments

@UlrichEckhardt
Copy link

I have a test comparing timestamps:

    assert.isAtLeast(modifiedAt, new Date(), 'now must not be before modifiedAt');

on failure, the output is:

  now must not be before modifiedAt
  + expected - actual

  -[Date: 2023-04-28T12:58:32.000Z]
  +[Date: 2023-04-28T12:58:33.113Z]

Using expected/actual is difficult here, because there is no expected value but a range of values. If you want to stick to that still, you could use this output instead:

  now must not be before modifiedAt
  + expected - actual

  -[Date: 2023-04-28T12:58:32.000Z]  < [Date: 2023-04-28T12:58:33.113Z]
  +[Date: 2023-04-28T12:58:32.000Z] >= [Date: 2023-04-28T12:58:33.113Z]

Maybe even drop the left comparand:

  now must not be before modifiedAt
  + expected - actual

  -  < [Date: 2023-04-28T12:58:33.113Z]
  + >= [Date: 2023-04-28T12:58:33.113Z]
@koddsson
Copy link
Member

Right the diff doesn't make sense here. I feel like the default message does hint towards how this is working but I agree.

      AssertionError: expected 2024-01-26T17:26:19.148Z to be at least 2024-01-26T17:26:19.150Z
      + expected - actual

      -[Date: 2024-01-26T17:26:19.148Z]
      +[Date: 2024-01-26T17:26:19.150Z]

@developer-bandi
Copy link
Contributor

@koddsson I try this job start, but there is a problem.

The behavior of the assert.isAtLeast function that I understand from the code is as follows.

function assertLeast (n, msg) {
if (msg) flag(this, 'message', msg);
var obj = flag(this, 'object')
, doLength = flag(this, 'doLength')
, flagMsg = flag(this, 'message')
, msgPrefix = ((flagMsg) ? flagMsg + ': ' : '')
, ssfi = flag(this, 'ssfi')
, objType = _.type(obj).toLowerCase()
, nType = _.type(n).toLowerCase()
, errorMessage
, shouldThrow = true;
if (doLength && objType !== 'map' && objType !== 'set') {
new Assertion(obj, flagMsg, ssfi, true).to.have.property('length');
}
if (!doLength && (objType === 'date' && nType !== 'date')) {
errorMessage = msgPrefix + 'the argument to least must be a date';
} else if (nType !== 'number' && (doLength || objType === 'number')) {
errorMessage = msgPrefix + 'the argument to least must be a number';
} else if (!doLength && (objType !== 'date' && objType !== 'number')) {
var printObj = (objType === 'string') ? "'" + obj + "'" : obj;
errorMessage = msgPrefix + 'expected ' + printObj + ' to be a number or a date';
} else {
shouldThrow = false;
}
if (shouldThrow) {
throw new AssertionError(errorMessage, undefined, ssfi);
}
if (doLength) {
var descriptor = 'length'
, itemsCount;
if (objType === 'map' || objType === 'set') {
descriptor = 'size';
itemsCount = obj.size;
} else {
itemsCount = obj.length;
}
this.assert(
itemsCount >= n
, 'expected #{this} to have a ' + descriptor + ' at least #{exp} but got #{act}'
, 'expected #{this} to have a ' + descriptor + ' below #{exp}'
, n
, itemsCount
);
} else {
this.assert(
obj >= n
, 'expected #{this} to be at least #{exp}'
, 'expected #{this} to be below #{exp}'
, n
);
}
}
Assertion.addMethod('least', assertLeast);
Assertion.addMethod('gte', assertLeast);
Assertion.addMethod('greaterThanOrEqual', assertLeast);

The Atleast function executes the assertLeast function, and if there is no type error, it executes this.assert.

chai/lib/chai/assertion.js

Lines 123 to 148 in 936c0ca

Assertion.prototype.assert = function (expr, msg, negateMsg, expected, _actual, showDiff) {
var ok = util.test(this, arguments);
if (false !== showDiff) showDiff = true;
if (undefined === expected && undefined === _actual) showDiff = false;
if (true !== config.showDiff) showDiff = false;
if (!ok) {
msg = util.getMessage(this, arguments);
var actual = util.getActual(this, arguments);
var assertionErrorObjectProperties = {
actual: actual
, expected: expected
, showDiff: showDiff
};
var operator = util.getOperator(this, arguments);
if (operator) {
assertionErrorObjectProperties.operator = operator;
}
throw new AssertionError(
msg,
assertionErrorObjectProperties,
(config.includeStack) ? this.assert : util.flag(this, 'ssfi'));
}
};

The assert function compares the actual value and the expected value to determine whether it passes or not, and then throws an error object when an error occurs.

When an error occurs, a message and information are formatted and displayed using a report library such as mocha.

If the above execution flow is correct, the change inside the assert function to correct the -+ error below is too large, so it seems that the assert function logic needs to be moved and modified inside the least function.

However, rather than changing it like this without showing the diff itself, how about including information that there is an error in the expected value in the error message returned if it is not a basic error message? for example,

  now must not be before modifiedAt [expected 2024-01-26T17:26:19.148Z to be at least 2024-01-26T17:26:19.150Z]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants