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.

Sign up for GitHub

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

Jump to bottom

Remove emulator + shims from testing samples #1255

Merged
merged 58 commits into from
Sep 20, 2019
Merged

Remove emulator + shims from testing samples #1255

merged 58 commits into from
Sep 20, 2019

Conversation

ace-n
Copy link
Contributor

@ace-n ace-n commented Apr 23, 2019 •
edited
Loading

DO NOT MERGE until:

  • the testing doc has been updated to not use the shim file (Googlers: cl/252452007 --> cl/269449711 )
  • the HTTP tests have been updated to use the Functions Framework.
  • docs have been updated to hide Node 6 tab (since this PR overwrites some Node 6 samples)
  • (Optional) Remove @google-cloud/nodejs-repo-tools from sample tests, as it's (AFAIK) primarily for our internal use.

@ace-n ace-n added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 23, 2019
@ace-n ace-n requested review from fhinkel and grant April 23, 2019 00:50
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 23, 2019
@ace-n ace-n force-pushed the deprecate-emulator branch from fd26f0a to 35c64f4 Compare April 23, 2019 00:59
grant
grant reviewed Apr 24, 2019
View reviewed changes
Copy link
Contributor

@grant grant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. Main comments:

  • The delays seem like they are unnecessary and would produce flaky tests. Let's remove them.

functions/helloworld/test/sample.integration.http.test.js Outdated Show resolved Hide resolved
functions/helloworld/test/sample.integration.http.test.js Outdated Show resolved Hide resolved
functions/helloworld/test/sample.integration.http.test.js Outdated Show resolved Hide resolved
functions/helloworld/test/sample.integration.pubsub.test.js Outdated Show resolved Hide resolved
functions/helloworld/test/sample.integration.pubsub.test.js Outdated Show resolved Hide resolved
functions/helloworld/test/sample.integration.pubsub.test.js Outdated Show resolved Hide resolved
functions/helloworld/test/sample.integration.pubsub.test.js Outdated Show resolved Hide resolved
functions/helloworld/test/sample.integration.storage.test.js Outdated Show resolved Hide resolved
@ace-n ace-n force-pushed the deprecate-emulator branch from c23dc56 to 3f1a818 Compare May 3, 2019 00:45
@fhinkel fhinkel mentioned this pull request Jun 5, 2019
Closed
grant
grant approved these changes Jun 10, 2019
View reviewed changes
Copy link
Contributor

@grant grant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine to me.
Not sure why the tests are failing.

functions/helloworld/test/sample.system.storage.test.js Outdated Show resolved Hide resolved
@ace-n ace-n added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 17, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 17, 2019
@ace-n
Copy link
Contributor Author

ace-n commented Sep 18, 2019

Update 3: switched from Node's childProcess timeouts to native UNIX ones (e.g. & sleep <TIMEOUT>; kill $!) since they were much cleaner to deal with on Linux.

@ace-n
Copy link
Contributor Author

ace-n commented Sep 18, 2019

Looks like the unit + integration tests (in functions/helloworld) are passing now. 🎉

Increased the timeout for the (currently failing) system tests - we'll see if that helps. 🙂

@ace-n
Copy link
Contributor Author

ace-n commented Sep 19, 2019 •
edited
Loading

Pub/Sub system tests are failing - I suspect due to a Stackdriver logs delay. Tests pass locally, so no choice but to debug in prod. 😦

Update: they're passing now. Any failures are likely due to Stackdriver flakiness/latency spikes.

@ace-n ace-n merged commit 963efc6 into master Sep 20, 2019
@ace-n ace-n removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 20, 2019
@ace-n ace-n changed the title [WIP] Remove emulator + shims from testing samples Remove emulator + shims from testing samples Sep 20, 2019
@ace-n ace-n deleted the deprecate-emulator branch September 20, 2019 17:49
@ricalo ricalo mentioned this pull request Oct 3, 2019
Merged
@snyk-bot snyk-bot mentioned this pull request Mar 11, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fhinkel fhinkel fhinkel left review comments

@grant grant grant approved these changes

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ace-n @grant @ricalo @fhinkel @santhoshvaddi @googlebot @kokoro-team