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

test: configure proper devDir for invoking configure() #1796

Closed
wants to merge 2 commits into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Jun 23, 2019

test/test-configure-python.js downloads a fresh set of headers to the package directory each time. By setting to the default global cache dir we get to re-use cached headers and skip the download step. Speeds up tests and prevents the creation of a new directory with headers in it in the package root each test run.

@richardlau
Copy link
Member

test/test-configure-python.js downloads a fresh set of headers to the package directory each time. By setting to the default global cache dir we get to re-use cached headers and skip the download step. Speeds up tests and prevents the creation of a new directory with headers in it in the package root each test run.

Just to double check -- we still download the headers at least once elsewhere in the tests?

@rvagg
Copy link
Member Author

rvagg commented Jun 25, 2019

@richardlau maybe not, maybe I'll add one to test-download.js which mocks it all out I think.

@richardlau
Copy link
Member

@richardlau maybe not, maybe I'll add one to test-download.js which mocks it all out I think.

Another way to check would be to see if coverage drops after this PR (now we're using tap).

@rvagg rvagg force-pushed the rvagg/cache-dir-for-python-test branch from 9b3f6d0 to ee46c5c Compare July 6, 2019 04:35
@rvagg rvagg force-pushed the rvagg/cache-dir-for-python-test branch from ee46c5c to c3dfcec Compare October 30, 2019 11:39
@rvagg
Copy link
Member Author

rvagg commented Oct 30, 2019

I've rebased this and added a whole new test that downloads headers and verifies contents. @nodejs/node-gyp ptal.

@rvagg rvagg force-pushed the rvagg/cache-dir-for-python-test branch from c3dfcec to 43fa2df Compare October 31, 2019 00:34
test/test-configure-python.js downloads a fresh set of headers to the
package directory each time. By setting to the default global cache dir
we get to re-use cached headers and skip the download step.

PR-URL: #1796
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
PR-URL: #1796
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@rvagg rvagg force-pushed the rvagg/cache-dir-for-python-test branch from 43fa2df to 0670e51 Compare November 18, 2019 02:31
@rvagg rvagg closed this Nov 18, 2019
@rvagg rvagg deleted the rvagg/cache-dir-for-python-test branch November 18, 2019 02:31
@rvagg
Copy link
Member Author

rvagg commented Nov 18, 2019

landed

rvagg added a commit that referenced this pull request Nov 18, 2019
test/test-configure-python.js downloads a fresh set of headers to the
package directory each time. By setting to the default global cache dir
we get to re-use cached headers and skip the download step.

PR-URL: #1796
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
rvagg added a commit that referenced this pull request Nov 18, 2019
PR-URL: #1796
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
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