-
Notifications
You must be signed in to change notification settings - Fork 830
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 a Windows path issue in the workbox-cli test. #884
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@@ -10,7 +10,7 @@ describe(`[workbox-cli] app.js`, function() { | |||
const PROXIED_CONFIG_FILE = path.resolve(process.cwd(), '/will/be/proxied'); | |||
const PROXIED_ERROR = new Error('proxied error message'); | |||
const PROXIED_CONFIG = {}; | |||
const INVALID_CONFIG_FILE = '/does/not/exist'; | |||
const INVALID_CONFIG_FILE = path.resolve(process.cwd(), '/does/not/exist'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will windows be ok with the forward slashes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
path.resolve()
converts them to whatever the OS uses, so it will be okay with this change in place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...at least it should, since I'm doing the same thing earlier, but this PR's AppVeyor run didn't complete due to #883
Re-running with #883 merged to confirm that it passes AppVeyor. |
PR-Bot Size PluginChanged File SizesNo file sizes have changed. New FilesNo new files have been added. All File SizesView Table
Workbox Aggregate Size PluginTotal Size: 8.35KB Gzipped: 3.81KB |
R: @philipwalton @gauntface
This should fix https://ci.appveyor.com/project/gauntface/sw-helpers/build/1.0.772#L353
Unfortunately, AppVeyor didn't re-run with my latest commits in the PR that introduced the test, so it wasn't picked up prior to merging.