sinonjs / referee Goto Github PK

We understand you have a problem and are in a hurry, but please provide us with some info to make it much more likely for your issue to be understood, worked on and resolved quickly.

I'm trying to use referee in a browser environment, more specifically in karma.
I'm loading the bundle version ('dist/referee.js')
But it crashes because of missing dependencies in the bundle

• library version : "@sinonjs/referee": "^3.2.0"
• Environment : karma 4.x
• Example URL : https://github.com/blikblum/slick-router/blob/use-open-wc-testing/karma.conf.js#L10
• Other libraries you are using:

What did you expect to happen?
referee bundle should work on browsers

What actually happens
referee crashes on browser

How to reproduce

Describe with code how to reproduce the faulty behaviour,
or link to code on JSBin or similar

Try to load bundle ('dist/referee.js') as a script on browser

This occurs because rollup does not bundle external dependencies by default. To do so is necessary to use 'rollup-plugin-node-resolve'

Let me know if a PR is accepted to make a bundle that works on browser

Passing a Boolean object, created using new Boolean() to assert.isBoolean() does not pass.

What did you expect to happen?

I imagine that the assert.isBoolean would pass, and that refute.isBoolean would fail.

I do think that creating Boolean values using new should be avoided, but it's part of the language, so we should probably handle it well.

See https://eslint.org/docs/rules/no-new-wrappers

What actually happens

assert.isBoolean fails, and refute.isBoolean passes

How to reproduce

var referee = require("referee");
var b = new Boolean(true);

// passes
referee.refute.isBoolean(b);

// fails
referee.assert.isBoolean(b);

In order to avoid situations where authors forget to add the expecation property, we can expand the API to have referee.addWithExpectation method.

This method should verify that the expectation property is set, and then delegate to referee.add.

Background

See #185

When upgrading from v2 to v3 of referee our tests fail with a ✖ ERROR: 'refuteMessage' option must be a non-empty String and nothing else (ref sinonjs/sinon-test#120). I went here to find a changelog, but could not find any (nor at the homepage).

Is there a way of easily browsing the breaking changes?

This package doesn't have a default file set. You can set it via jsdelivr, browser, or main field in package.json

referee.count is an artefact from buster, which was made so that buster could report on how many assertions were run, and could fail a test that had no assertions.

I don't know of any other test runner that has this feature or uses referee.count for anything.

Since master is already primed for another MAJOR release by merging #46, I think now would be a good time to remove referee.count.

Opinions?

As an author, it is currently rather inconvenient to add custom assertions to referee.

I always end up creating some meta file and importing that

// referee-with-custom-assertions.js
referee.add(/* arguments */);

module.exports = referee;

// some-module.test.js
var referee = require('../test/referee-with-custom-assertions');

This doesn't make it very convenient for me to re-use my domain specific assertions easily across repositories.

It would be really handy, if I could add custom assertions to referee via plugins, in the same fashion as plugins for eslint.

My custom assertions could then be published to an internal registry (or the public registry), and re-used across projects.

// referee.config.js
module.exports = {
    plugins: [
        require('path/to/max/assertions'),
        require('morgan-assertions')
    ]
};

// some-module.test.js
var referee = require('referee');

Thoughts?

I was doing some work with async functions, and was finding myself having to write something like this to verify the error handling.

try {
    await myAsyncFunction();
} catch (e) {
    error = e;
} finally {
    assert(error instanceof TypeError);
    assert.equals(
        error.message,
        'Oops'
    );
}

This seems clumsy, compared to a lot of the things we can do with referee.

I'd prefer to write something like this:

// option 1: expand assert.exception to cover promises
assert.exception(myAsyncFunction(), {name: 'TypeError', message: 'Oops'})

// option 2: adding extra methods 
assert.rejects(myAsyncFunction(), {name: 'TypeError', message: 'Oops'});

Considering that we have sinon.fake.resolves() and sinon.fake.rejects(), I think I'd prefer option 2 and add assert.resolves() and assert.rejects().

I imagine that assert.rejects follows the API established with assert.exception, except for the fact that it understands promises.

When I'm testing the DOM, I'd like to have more assertions, for name, value, checked etc. Referee is also used on Node and I find it a bit messy to have DOM related checks in core.

We have the possibility to create sets of custom assertions. I would like to create a new repository for them and move the functionality there.

We understand you have a problem and are in a hurry, but please provide us with some info to make it much more likely for your issue to be understood, worked on and resolved quickly.

• library version : 10.0.0
• Environment : OSX
• Other libraries you are using: sinon

How to reproduce

assert.exception(
    function () {
        throw new TypeError('long exception message');
    },
    {
        message: "message",
        name: "rr",
    }
);

What did you expect to happen?

Test fails

What actually happens

Test succeeds

Reason

When samsam library detects there's no match, referee tries to verify that by iterating property-by-property and comparing again: https://github.com/sinonjs/referee/blob/main/lib/assertions/exception.js#L52

The problem is, comparing objects is different from comparing strings in samsam.

When iterating property-by-property, we don't check for string equality, rather that one string is a substring of anoter: https://github.com/sinonjs/samsam/blob/main/lib/match.js#L71-L74

This makes the check in assert.exception weaker - it succeeds if the strings in the matcher strings are just the substrings of the corresponding properties.

I would expect this behaviour to be either changed or explicitly documented.

Motivation:

It would be totally awesome if this was possible:

const { assert, match } = require('@sinonjs/referee');

const object = { my: 'thingy' };

assert.equals(object, { my: match.string });

Suggestion

A newly exposed match function could essentially behave like sinon.match. I think the matchers actually belong in samsam and the deepEqual logic in samsam and sinon should be unified with matcher support. This way sinon, @sinonjs/referee and @sinonjs/referee-sinon can expose it and have the same semantics everywhere.

It would then be possible to pass the result of calling match({ some: 42 }) to both, assert.equals and spy.calledWith.

The assert.rejects function currently requires a second argument, which much be identical to what the target function rejected with. This can be difficult to determine in many cases. It would be great if the second argument was optional (for what it's worth, the docs say the second param is optional here, but maybe these aren't up to date: https://sinonjs.github.io/referee/#rejects). Here's an example (using mocha) of how I'd like this to work:

const {assert} = require('@sinonjs/referee');

async function thrower() {
    throw new Error('foo');
}

describe("assert.reject", () => {
    it("does not require a second argument", async () => {
        await assert.rejects(thrower());
    });
});

Or perhaps it could follow the same api as assert.match(). Then I could do something like:

await assert.rejects(thrower(), /foo/);

Code

• Configure mocha and referee
• Port tests to mocha
• Use nyc for test coverage
• Add .editorconfig
• ESLint
  • Use eslint-config-sinon
  • Use eslint-plugin-ie11
  • Use eslint-plugin-mocha
  • Use lint-staged and husky for pre-commit hook

Third party services

• Run tests in node LTS versions (4, 6, 8) on Travis
• Push coverage report to coveralls
• Configure bithound.io for repository

Repository

• Port documentation to MkDocs and publish to GitHub Pages
• Add CONTRIBUTING.md
• Add pull request template
• Add new issue template
• Add .github/stale.yml and create pinned and stale labels
• Protect master branch from force pushes, including for administrators

NPM

• Add files directive to filter out files that package consumers would not want (#17)
• Publish module under @sinonjs scope with a new version
• Deprecate old package on npm (@mroderick can do this)

If updating the referee version from 2.0.0 to 2.1.0 in Sinon, then running the test-headless script fails. I am guessing proxyquirify has problems handling the dynamic imports that were introduced.

> [email protected] test-headless /home/carlerik/dev/sinon
> mochify --recursive -R dot --grep WebWorker --invert --plugin [ proxyquire-universal ] test/**.js

# chromium:
Evaluation failed: Error: Cannot find module './assertions/defined'
      at newRequire (node_modules/proxyquireify/lib/prelude.js:52)
      at req (node_modules/proxyquireify/lib/prelude.js:61)
      at moduleRequire (node_modules/proxyquireify/lib/prelude.js:71)
      at node_modules/@sinonjs/referee/lib/referee.js:239
    at Array.forEach (<anonymous>)
      at Object.require.50../assert-arg-num (node_modules/@sinonjs/referee/lib/referee.js:238)
      at newRequire (node_modules/proxyquireify/lib/prelude.js:75)
      at req (node_modules/proxyquireify/lib/prelude.js:61)
      at moduleRequire (node_modules/proxyquireify/lib/prelude.js:71)
      at Object.<anonymous> (test/assert-test.js:4)
Error: Exit 1

• library version : 4.0.0
• Environment : Node 13.5.0
• Example URL : https://sinonjs.github.io/referee/#defined
• Other libraries you are using: Mocha 7.0.0

What did you expect to happen?
Documentation should be up to date.
Error messages from assertions should be accurate.

What actually happens
Documentation still lists the predefined assert defined, and not isUndefined.
Error message from predefined assert equals gives the following error message if the expectiation value is undefined:

AssertionError: [assert.equals] Expectation for equals should not be undefined. Use assert.defined or refute.defined instead.

How to reproduce
assert.equals('not undefined', undefined)

• library version : 3.2.0
• Environment : Node

What did you expect to happen?
That it would print a normal assertion error

What actually happens
It fails with

TypeError: Cannot convert object to primitive value
    at String (<anonymous>)
    at /tmp/test1/node_modules/@sinonjs/referee/lib/interpolate-pos-arg.js:12:44
    at Array.reduce (<anonymous>)
    at interpolatePosArg (/tmp/test1/node_modules/@sinonjs/referee/lib/interpolate-pos-arg.js:9:12)
    at Object.fail (/tmp/test1/node_modules/@sinonjs/referee/lib/define-assertion.js:30:27)
    at assertion (/tmp/test1/node_modules/@sinonjs/referee/lib/define-assertion.js:68:17)
    at Function.referee.<computed>.<computed> [as isUndefined] (/tmp/test1/node_modules/@sinonjs/referee/lib/define-assertion.js:93:26)
    at Object.<anonymous> (/tmp/test1/mytest.js:4:8)

How to reproduce

$ cat > mytest.js  << EOF
var referee = require("@sinonjs/referee");
var assert = referee.assert;

assert.isUndefined(Object.create(null));
EOF

node mytest.js 

We should replace the internal helper multiLineStringDiff with diff and color. Both modules are well established, work in our target runtimes and are easy to use.

As can be seen in the code coverage report, the existing code is not covered by tests

Background

Acceptance criteria

• works in node
• works in browsers
• has tests :)

The resolves and rejects assertions diverge from the naming convention use by Sinon assertions. In Sinon, resolves and rejects are used to program a stub or a fake, e.g. sinon.fake.resolves(42). The Sinon assertions to verify promises are sinon.assert.resolved(...) and sinon.assert.rejected(...).

I would therefore like to propose changing the names as follows:

• resolves -> resolved
• rejects -> rejected
• toResolveWith -> toHaveBeenResolved
• toRejectWith -> toHaveBeenRejected

If this is doable, we can retire @sinonjs/formatio, which would cut down on tedious maintenance

https://github.com/browserify/node-util

We don't have to worry (too much) about bundle size, since none of the Sinon projects are for production use, but only for test environments.

To my surprise, the following passes:

assert.match(
  { array: [1, 2, 3], more: true },
  { array: [2] }
)

My expectation would be that the above would fail due to the array not being equal. The following should pass:

assert.match(
  { array: [1, 2, 3], more: true },
  { array: [1, 2, 3] }
)

This differs from how Sinon.JS matchers work:

const spy = sinon.spy()
spy({ array: [1, 2, 3], more: true })

spy.calledWithMatch({ array: [1, 2, 3] }) // true
spy.calledWithMatch({ array: [2] }) // false

I prefer the Sinon match logic, because it allows me to explicitly state what I expect with nested matchers:

spy.calledWithMatch({ array: sinon.match.array.contains([2]) })

If we're merging the matcher logic, I think we should basically replace the referee logic with the Sinon implementation, especially since the user base is a tad larger over there.

As suggested by @mroderick in #87 we should introduce isUndefined and deprecate defined to align with isNull, isTrue, etc.

Extracted form the discussion of #47.

#47 (comment)

* If #45 lands, then expand `verify` to ensure all promises from `resolves` and `rejects` are resolved or rejected

The following example will pass, because we don't let the test runner handle the returned promise.

describe("some test", function() {
    it("should resolve", function() {
        referee.assert.resolves(Promise.resolve(42), 43);
    }):
});

This issue is an approach to verify if all async assertions have been properly resolved.

@mroderick, @mantoni

Symmetric assert/refute (refute is referee's "assert.not*")

🤔 "assert" and "refute" aren't "symmetrical"...

"Refute" (demonstrate that something is false) isn't the opposite of "assert" (state that something is true). It's the opposite of "prove" (demonstrate that something is true). The opposite of "assert" is "deny" (state that something is false).

I guess it's too late to change it, but perhaps it could be added as an alias?

(I realise the name comes from minitest, and the noun ("refutations") sounds like a better complement to "assertions" (to me) than "denials".)

This should fail, but is passes:

assert.match({}, { unknown: undefined });