lawrencec / Unroll

A helper tool to easily run the same tests against multiple data with verbose output.
16 stars 5 forks source link

Accept null or undefined as value #53

Open isaacaggrey opened 6 years ago

isaacaggrey commented 6 years ago

Current behavior:

unroll('should be okay with id being #id', (done, testArgs) => {
    let object = { id: testArgs.id };
    expect(object.id).toEqual(testArgs.id);
    done();
},
`
    where:
    id
    ${null}
    ${undefined}
`
);

// Output:
    ✖ encountered a declaration exception
    Error: title not expanded as incorrect args passed in

Expected behavior:

✔ should be okay with id being null
✔ should be okay with id being undefined

Granted, I understand with the dataTable method this is a bit tricky since it is a template literal (i.e., what if someone wants to test the string "null" or "undefined", not sure what is expected behavior), but I think the dataArray method should be valid.

lawrencec commented 6 years ago

Hey @isaacaggrey, Thanks for reporting the bug!

I agree this should be supported. I have a fix for both the array and datatable notations using Symbols. So for example, your dataTable example will look like this:

unroll('should be okay with id being #id', (done, testArgs) => {
    let object = { id: testArgs.id };
    expect(object.id).toEqual(testArgs.id);
    done();
},
`
    where:
    id
    ${Symbol.keyFor(unroll.NULL)}
    ${Symbol.keyFor(unroll.UNDEFINED)}
`
);

and the array notation:

unroll('should be okay with id being #id', (done, testArgs) => {
    let object = { id: testArgs.id };
    expect(object.id).toEqual(testArgs.id);
    done();
},
[
   ['id']
   [null],
   [undefined],
   [Unroll.NULL],
   [Unroll.UNDEFINED],
]

Admittedly, the dataTable example is ugly (though one could argue it's a little messy anyway cos of the need for JSON.stringify), this is because unroll needs to know that the undefined is intentional so a placeholder replacement has to be used as an identifier. And I'd like to keep functional parity between the two notations; I wouldn't want to be able to use null and undefined in array notation but not dataTables.

But I don't think there is an easier way of representing undefined ornull` without removing the check (and subsequent thrown error) that the test title has not been expanded properly. The alternative is to get rid of the error checking but I think that's useful for the developer.

Would be interested in your thoughts and preferences as an end user. AFAIK, I'm the main user of the util so would be good to get other users opinion before I push this.

Blackbaud-JasonBodnar commented 5 years ago

@lawrencec any plans to merge this PR?