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

Add Jest helper and support #290

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

adrianmcli
Copy link

This PR fixes #289 and fixes #281.

In this PR, I've taken a first stab at adding support for Jest. The reasons for this is discussed in #289. This PR is heavily inspired by @jayandcatchfire and the work shown in #281.

So far, I have done the following:

  • Add the helper function created by @jayandcatchfire
  • Add a simple test co-located with the helper file
  • Add Jest to the testing pipeline in the Makefile
  • Add documentation to describe how to use the helper

I have run make test and make literate, and both commands yield the expected results.

However, I'm not sure how users will be requiring the helper and I'm not sure how to push the helper into the build pipeline. It would be good to get some guidance on this, perhaps @phadej can provide some guidance?

Thanks for your time :)

@hexpunk
Copy link

hexpunk commented Jan 31, 2019

This is all very cool. I'd just like to point out that none of this is exclusively applicable to Jest. It should apply to any BDD testing system that exposes an it function to the global scope, such as Chai (when configured to export it to the global scope such as some plug-ins for Karma do).

@adrianmcli
Copy link
Author

@jayandcatchfire If you are interested, I can give you collaborator privileges over at my fork and we can work on this together to get it approved by maintainers of this repo.

@adrianmcli
Copy link
Author

An invite has been sent :)

I'm happy to generalize this for any BDD frameworks like you have suggested, but I'll need some guidance here.

* @param testFn function that expects, in proper jest syntax
* @param opts options for jsverify
*/
function itHolds(description, arbitrary, testFn, options) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

single arbitrary only?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reworked version supports everything that jsc.property supports since it just wraps that.

```js
describe("a simple jest test suite", function () {
itHolds("(b && b) === b", jsc.bool, function (b) {
expect(b && b).toBe(b);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand, isn't it's simpler to write

describe("a simple jest test suite", function () {
  itHolds("(b && b) === b", jsc.bool, function (b) {
    expect(b && b).toBe(b);
    return true; // if you forget this your test will fail, so it's not a silent error
  });
});

Does jest support asynchronous tests (returning promise or calling callback), how these work?

Can't itHolds be implemented using jsc.property?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jayandcatchfire maybe you can answer this?

Copy link

@hexpunk hexpunk Jan 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jest does support asynchronous tests by means of passing an asynchronous function to it. I'm not sure how/if that would work with jsverify. I'll have to dig a little bit.

As for using jsc.property, that's not how I originally wrote my helper, but I'll play with it and see if that would work. Keep in mind I was using typescript, so I wasn't concerned about using the DSL.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for helping out on this @jayandcatchfire

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I played a bit with jsc.property. The reason I originally avoided it was because I found the documentation a bit ambiguous. Now that I've read its source and played with it, it appears that the only important difference between my helper and jsc.property is that mine returns true at the end so the test doesn't fail even when nothing throws in the test function.

I just rewrote my function as the following and it appears to work in all the existing cases I've tried.

function itHolds(...args: any[]) {
  const initial = args.slice(0, -1);
  const prop = args[args.length - 1];

  jsc.property(...initial, (...args: any[]) => {
    prop(...args);

    return true;
  });
}

Of course, this was quick and dirty. I haven't accounted for any of the types here, which is bad form.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's my final version that retains proper typing:

import * as jsc from 'jsverify';

/* tslint:disable:max-line-length */
export function itHolds<A>(description: String, arb1: jsc.Arbitrary<A>, prop: (t: A) => jsc.Property<A>): any;
export function itHolds<A, B>(description: String, arb1: jsc.Arbitrary<A>, arb2: jsc.Arbitrary<B>, prop: (t: A, u: B) => jsc.Property<any>): any;
export function itHolds<A, B, C>(description: String, arb1: jsc.Arbitrary<A>, arb2: jsc.Arbitrary<B>, arb3: jsc.Arbitrary<C>, prop: (t: A, u: B, v: C) => jsc.Property<any>): any;
export function itHolds<A, B, C, D>(description: String, arb1: jsc.Arbitrary<A>, arb2: jsc.Arbitrary<B>, arb3: jsc.Arbitrary<C>, arb4: jsc.Arbitrary<D>, prop: (t: A, u: B, v: C, w: D) => jsc.Property<any>): any;
export function itHolds<A, B, C, D, E>(description: String, arb1: jsc.Arbitrary<A>, arb2: jsc.Arbitrary<B>, arb3: jsc.Arbitrary<C>, arb4: jsc.Arbitrary<D>, arb5: jsc.Arbitrary<E>, prop: (t: A, u: B, v: C, w: D, e: E) => jsc.Property<any>): any;
export function itHolds<A, B, C, D, E, F>(description: String, arb1: jsc.Arbitrary<A>, arb2: jsc.Arbitrary<B>, arb3: jsc.Arbitrary<C>, arb4: jsc.Arbitrary<D>, arb5: jsc.Arbitrary<E>, arb6: jsc.Arbitrary<F>, prop: (t: A, u: B, v: C, w: D, e: E, a: F) => jsc.Property<any>): any;
export function itHolds<A, B, C, D, E, F, G>(description: String, arb1: jsc.Arbitrary<A>, arb2: jsc.Arbitrary<B>, arb3: jsc.Arbitrary<C>, arb4: jsc.Arbitrary<D>, arb5: jsc.Arbitrary<E>, arb6: jsc.Arbitrary<F>, arb7: jsc.Arbitrary<G>, prop: (t: A, u: B, v: C, w: D, e: E, a: F, b: G) => jsc.Property<any>): any;
export function itHolds<A, B, C, D, E, F, G, H>(description: String, arb1: jsc.Arbitrary<A>, arb2: jsc.Arbitrary<B>, arb3: jsc.Arbitrary<C>, arb4: jsc.Arbitrary<D>, arb5: jsc.Arbitrary<E>, arb6: jsc.Arbitrary<F>, arb7: jsc.Arbitrary<G>, arb8: jsc.Arbitrary<H>, prop: (t: A, u: B, v: C, w: D, e: E, a: F, b: G, c: H) => jsc.Property<any>): any;
export function itHolds<A, B, C, D, E, F, G, H, I>(description: String, arb1: jsc.Arbitrary<A>, arb2: jsc.Arbitrary<B>, arb3: jsc.Arbitrary<C>, arb4: jsc.Arbitrary<D>, arb5: jsc.Arbitrary<E>, arb6: jsc.Arbitrary<F>, arb7: jsc.Arbitrary<G>, arb8: jsc.Arbitrary<H>, arb9: jsc.Arbitrary<I>, prop: (t: A, u: B, v: C, w: D, e: E, a: F, b: G, c: H, d: I) => jsc.Property<any>): any;
export function itHolds<A, B, C, D, E, F, G, H, I, J>(description: String, arb1: jsc.Arbitrary<A>, arb2: jsc.Arbitrary<B>, arb3: jsc.Arbitrary<C>, arb4: jsc.Arbitrary<D>, arb5: jsc.Arbitrary<E>, arb6: jsc.Arbitrary<F>, arb7: jsc.Arbitrary<G>, arb8: jsc.Arbitrary<H>, arb9: jsc.Arbitrary<I>, arb10: jsc.Arbitrary<J>, prop: (t: A, u: B, v: C, w: D, e: E, a: F, b: G, c: H, d: I, f: J) => jsc.Property<any>): any;
/* tslint:enable:max-line-length */

/**
 * wrapper function for using jsverify with BDD syntax
 */
export function itHolds(...args: any[]): any {
  const initial = args.slice(0, -1);
  const prop = args[args.length - 1];

  jsc.property(...initial, (...args: any[]): boolean => {
    prop(...args);

    return true;
  });
}

I still don't know if/how this would support async tests.

@adrianmcli
Copy link
Author

Just fyi @jayandcatchfire, you have access to push direct changes to this PR.

@hexpunk
Copy link

hexpunk commented Jan 31, 2019

Just fyi @jayandcatchfire, you have access to push direct changes to this PR.

I'm not sure of the best way to apply my changes for two reasons.

  1. I'm using the ES6 convention of the spread operator liberally. I can change mine to support ES5 and use the arguments object, although MDN warns that it's deprecated. I'd bet that would be @phadej's preference, though, as that's what is used in the code for jsc.property.

  2. I'm using Typescript. Although I'd love for this helper to still support types, I don't know how we'd do that since the main library doesn't export helpers directly and it provides types via a .d.ts file.

@phadej, can you weigh in on how you'd prefer this to work?

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

Successfully merging this pull request may close these issues.

Add guidelines and support for usage with Jest Suggestion for BDD style test helper
3 participants