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

fix: stop uploading directories by passing nodir=true to glob #174

Merged
merged 1 commit into from Dec 2, 2019

Conversation

@karszawa
Copy link
Contributor

karszawa commented Dec 2, 2019

Thanks for such a cool tool! I have met an issue while trying to use this tool. I'm happy if you could see this change 😊

What does this change?

A brief description of the issue

While publishing files, this tool tries to upload directories under the hard-to-happen condition. That is the case when a directory ends in file extension like .js, .html, wasm, and so on. It happens because files are listed by node-glob by a bit looser condition like the example below (this is not a complete copy of the existing code, though)

glob("**/*.{html,js,wasm,png,json,jpeg,jpg,tiff,bmp,gif}", {
 cwd: this.getWorkingDirs().base,
 }, ...

Under this condition, directories that have the name with file extension can be listed. As a result, file uploading would fail.

How did I solve it

It's easy to fix it because node-glob has an option to exclude directories, like nodir. I am just passing it as true!

I don't know we should have DEFAULT_PATTERN as it is because the motivation for this variable seems that you guys wanted to exclude directories. For now, it can be just "**/*"? (Maybe not, but I want to know the motivation for this code just for my curiosity)

More detailed story

I'm using Cypress for E2E tests in my project, and I wanted to try visual regression testing by this tool. In Cypress, it has a built-in screenshot function, which saves files into cypress/snapshots/[testfilename]/testname.png by default. Because of this Cypress's default naming rule, a directory is normally named with a file extension like .js.

I tried GCS to save files, and it had raised an error like this.

$ npx reg-suit run
[reg-suit] info version: 0.8.4
[reg-suit] warn Failed to detect the previous snapshot key
[reg-suit] info Skipped to fetch the expected data because expected key is null.
[reg-suit] info Comparison Complete
[reg-suit] info    Changed items: 0
[reg-suit] info    New items: 1
[reg-suit] info    Deleted items: 0
[reg-suit] info    Passed items: 0
[reg-suit] info The current snapshot key: '2453ec3a853641550dc348d72b243f96fdd3eaf7'
                                         ■ 0% | ETA: 0s | 0/6[reg-publish-gcs-plugin] info Upload 6 files to reg-publish-bucket-9e541de8-585d-47a6-ac1e-8b7ce4f48e05.
[reg-suit] error An error occurs during publishing snapshot:
[reg-suit] error  { [Error: EISDIR: illegal operation on a directory, read] errno: -21, code: 'EISDIR', syscall: 'read' }
{ [Error: EISDIR: illegal operation on a directory, read] errno: -21, code: 'EISDIR', syscall: 'read' }

That is the original motivation of this Pull Request.

Screenshots

No screenshots needed to check this change

What can I check for bug fixes?

  • the current implementation causes the error while uploading images under screenshots/hoge.js (thus, the file structure should be screenshots/hoge.js/foo.png
  • the suggested change fix does not cause the error
@Quramy Quramy merged commit 9d7c875 into reg-viz:master Dec 2, 2019
@Quramy

This comment has been minimized.

Copy link
Member

Quramy commented Dec 2, 2019

Thanks 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.