search

sindresorhus / gulp-rev

Static asset revisioning by appending content hash to filenames: `unicorn.css` → `unicorn-d41d8cd98f.css`
MIT License
1.54k stars 217 forks source link

Make tests independent of platform specific path separator (#219) #221

Closed WVmf closed 7 years ago

WVmf commented 7 years ago

Resolves #219

Most of the path module's method will replace forward and backward slashes with the platform specific path separator. And gulp-rev enforces the use of forward slashes for the manifest. The tests didn't take these two behaviors into account, which resulted in mismatches for some test on platforms that use backward slashes as the path separator (i.e. the Windows platform).

These mismatches are resolved by this patch by using path.normalize() and .replace(/\\/g, '/') at the proper places in the tests.

WVmf commented 7 years ago

Hi @Zhouzi,

These changes are indeed limited to the test files and don't change anything in the index.js I encountered the problems with the test files while trying to create a fix for issue #220. For which I'll create another pull request after I get some feedback on that issue regarding the possible valid values for a vinyl file's cwd, base and path properties.

sindresorhus commented 7 years ago

@Zhouzi LGTY now?

zhouzi commented 7 years ago

@sindresorhus just merged, might need a new release.

sindresorhus commented 7 years ago

Is it a breaking change?

zhouzi commented 7 years ago

Nope 👌

sindresorhus commented 7 years ago

Actually, it is a major release because of https://github.com/sindresorhus/gulp-rev/commit/8686e62b380a22976f314687bd3f79ca69cae24f.

WVmf commented 7 years ago

Hi, Thank you for merging the changes. Regarding the new release, maybe postpone it a little more because I was planning another pull request for #220.

WVmf commented 7 years ago

Ok just saw the new release was already made, so disregard the postponing part of my previous comment :)