Skip to content
This repository has been archived by the owner on Jun 11, 2020. It is now read-only.

Assert registry directory is subset, not deep equal #519

Closed
wants to merge 1 commit into from

Conversation

sandersn
Copy link
Member

Fixes #518

@sandersn sandersn requested a review from a user November 16, 2018 18:57
const expectedStat = await stat(expectedFile);
const actualStat = await stat(actualFile);
assert.strictEqual(expectedStat.isDirectory(), actualStat.isDirectory());
if (expectedStat.isDirectory()) {
await assertDirectoriesEqual(expectedFile, actualFile, options);
await assertDirectoryIsSubset(expectedFile, actualFile, options);
} else {
assert.strictEqual(await readFile(actualFile), await readFile(expectedFile));
Copy link

Choose a reason for hiding this comment

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

This is still asserting that the file contents are exactly equal. The set of files that exist in types-registry doesn't change, but the content of index.json does.

Copy link
Member Author

Choose a reason for hiding this comment

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

So is the right fix just to add "index.json" to validate's ignore function?

Copy link
Member Author

Choose a reason for hiding this comment

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

Or should I change the function to read specific (JSON) files and pass them on to another function, which then makes a subset assertion about JSON instead of file structure?

Copy link

@ghost ghost Nov 17, 2018

Choose a reason for hiding this comment

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

assertDirectoriesEqual is a red herring because index.json is the only really important file in types-registry. Though we do want to assert that no extra files somehow made there way there and the package.json isn't corrupted somehow.

We should assert that the published index.json doesn't contain anything it shouldn't, so its contents should be a subset of what a fully up-to-date index.json would contain.

It might also be nice to assert that the published index.json does contain everything it should, which means every package more than a week old. But that's less important to fix right now.

Copy link

Choose a reason for hiding this comment

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

Since types-registry only contains package.json, index.json, and README.md, having a recursive assertDirectoriesEqual utility may be overkill.

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

Successfully merging this pull request may close these issues.

1 participant