Skip to content

Structure fixtures so they can be easily tested for exclusivity #215

Merged
sindresorhus merged 2 commits intosindresorhus:mainfrom
bjornstar:test-improve-excludes
Jun 6, 2025
Merged

Structure fixtures so they can be easily tested for exclusivity #215
sindresorhus merged 2 commits intosindresorhus:mainfrom
bjornstar:test-improve-excludes

Conversation

@bjornstar
Copy link
Copy Markdown
Contributor

Without extensive exclude declarations as specified in the TODO message:

// TODO: Automatically exclude value types in other tests that we have in the current one.
// Could reduce the use of `exclude`.

I identified each unique entry in the fixtures and, if they were reused across multiple types, hoisted them to the top and spread them into the fixtures. This allows us to filter out fixture values that have already been tested as being of the type under test. It becomes more declarative in the fixture definitions, rather than in the call site of the test.

I split the primitive type fixtures from the object type fixtures so that we can exclude all the entries that are not primitives without needing redundant properties in the fixture data.

Another structural change to the fixtures was changing the fixtures from a Map to a readonly object so that we can use the keys to access the is methods that we are testing. This removed 2 redundant lines from each fixture.

I also iterate over the method names defined in the fixtures to ensure that everything listed is tested. Previously every fixture was used manually and needed to be added to a test after it was created.

If a method name has additional tests beyond the exclusive fixture based test, I labelled it as supplemental.

There was one tricky bit in there with Buffer and Uint8Array being the same thing so I added an exclusion there.

⚠️ There was one small functional change that I added regarding the Promise type. I think we should prefer the Promise type over Object when it has the promise API. It seems strange that we would expect the typeDescription to be Promise, but the typeName would be Object. If you think we should leave it as is, I can probably create an exclusion for it.

Comment thread source/index.ts Outdated
@sindresorhus
Copy link
Copy Markdown
Owner

There was one small functional change that I added regarding the Promise type. I think we should prefer the Promise type over Object when it has the promise API. It seems strange that we would expect the typeDescription to be Promise, but the typeName would be Object. If you think we should leave it as is, I can probably create an exclusion for it.

I agree, that's more correct.

@sindresorhus sindresorhus merged commit ef35cc3 into sindresorhus:main Jun 6, 2025
1 check passed
@sindresorhus
Copy link
Copy Markdown
Owner

Thanks :)

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.

2 participants