-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[DO NOT MERGE] Update/add tests + fix dependencies #452
Conversation
Codecov Report
@@ Coverage Diff @@
## master #452 +/- ##
==========================================
- Coverage 83.92% 83.84% -0.08%
==========================================
Files 4 4
Lines 423 421 -2
==========================================
- Hits 355 353 -2
Misses 68 68
Continue to review full report at Codecov.
|
Note/Reminder: |
appengine/cloudsql/package.json
Outdated
"@google-cloud/nodejs-repo-tools": "1.4.15", | ||
"ava": "0.19.1" | ||
}, | ||
"cloud-repo-tools": { |
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.
tldr; from our convo, revert changes to "cloud-repo-tools" and the 2 new .js test files, and instead update circle.yml
appengine/pubsub/app.js
Outdated
const process = require('process'); // Required for mocking environment variables | ||
|
||
// Make sure starting directory is consistent, so that views are found correctly | ||
process.chdir(__dirname); |
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.
Instead of this wonkiness, explicitly tell express where to look for views.
@@ -2,16 +2,17 @@ | |||
"name": "appengine-storage", | |||
"description": "Node.js Google Cloud Storage sample for Google App Engine", | |||
"scripts": { | |||
"start": "node app.js" | |||
"start": "node app.js", | |||
"test": "ava system-test/*.test.js -T 30s" |
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.
Change this to:
"test": "GOOGLE_STORAGE_BUCKET=$(uuidgen) samples test app && ava system-test/*.test.js -T 30s"
AND
add a "cloud-repo-tools" to the package.json that includes a "requiredEnvVars" clause
AND
delete your config file
AND
Add a line to circle.yml to invoke this test
AND
apply these comments to other appengine apps that you modified.
Can potentially close this in favor of #463 |
This was replaced by #468. |
* build!: Update library to use Node 12 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* build!: Update library to use Node 12 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* build!: Update library to use Node 12 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* build!: Update library to use Node 12 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Do not merge until this PR is included in a tagged version of
nodejs-repo-tools
. (The tests will fail otherwise.)Also - this is definitely not the last PR on this topic. (There will probably be 2-4 more as I context-switch between things.)