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

functions/billing: test failure with non-actionable messages #1565

Closed
grayside opened this issue Dec 12, 2019 · 8 comments
Closed

functions/billing: test failure with non-actionable messages #1565

grayside opened this issue Dec 12, 2019 · 8 comments
Assignees
Labels
api: cloudfunctions Issues related to the Cloud Run functions API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. 🚨 This issue needs some love. samples Issues that are directly related to samples. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@grayside
Copy link
Collaborator

Two tests have this same failure state, which prevents effective triage.

  1. The tests need to be fixed
  2. The test needs to surface the underlying error.

Currently blocking merge on #1515

  functions/billing tests
    notifies Slack
      functions_billing_slack
        ✓ should notify Slack when budget is exceeded (868ms)
    disables billing
      functions_billing_stop
(node:61) UnhandledPromiseRejectionWarning: ChildProcessError: Command failed: functions-framework --target=notifySlack --signature-type=event --port 8080
 `functions-framework --target=notifySlack --signature-type=event --port 8080` (exited with error code null)
    at callback (/tmpfs/src/github/nodejs-docs-samples/functions/billing/node_modules/child-process-promise/lib/index.js:33:27)
    at ChildProcess.exithandler (child_process.js:288:5)
    at emitTwo (events.js:126:13)
    at ChildProcess.emit (events.js:214:7)
    at maybeClose (internal/child_process.js:915:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:209:5)
(node:61) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:61) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
        1) should disable billing when budget is exceeded
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Dec 13, 2019
@fhinkel fhinkel added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Dec 20, 2019
@fhinkel
Copy link
Contributor

fhinkel commented Dec 20, 2019

@ace-n Can you have a look at this functions tests please. Thanks

@JustinBeckwith JustinBeckwith added the api: cloudfunctions Issues related to the Cloud Run functions API. label Feb 15, 2020
@fhinkel fhinkel added priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Mar 24, 2020
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Mar 24, 2020
@ace-n
Copy link
Contributor

ace-n commented May 21, 2020

Closing this as outdated as #1515 has since been merged.

@fhinkel @grayside please reopen if I'm wrong. Thanks! 🙂

@ace-n ace-n closed this as completed May 21, 2020
@grant grant reopened this Sep 28, 2020
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Sep 28, 2020
@grant
Copy link
Contributor

grant commented Sep 28, 2020

@ace-n Seeing this error consistently and am unable to merge an unrelated PR: #1986

1 failing

  1) functions/billing tests
       disables billing
         functions_billing_stop
           should disable billing when budget is exceeded:

      AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:

  assert.ok(response.body.includes('Billing disabled'))

      + expected - actual

      -false
      +true

      at Context.it (test/index.test.js:131:16)
      at process._tickCallback (internal/process/next_tick.js:68:7)



(node:54) UnhandledPromiseRejectionWarning: ChildProcessError: Command failed: functions-framework --target=stopBilling --signature-type=event --port 8081
Error: Invalid Credentials
    at Gaxios._request (/tmpfs/src/github/nodejs-docs-samples/functions/billing/node_modules/gaxios/build/src/gaxios.js:89:23)
    at process._tickCallback (internal/process/next_tick.js:68:7)
 `functions-framework --target=stopBilling --signature-type=event --port 8081` (exited with error code null)
    at callback (/tmpfs/src/github/nodejs-docs-samples/functions/billing/node_modules/child-process-promise/lib/index.js:33:27)
    at ChildProcess.exithandler (child_process.js:301:5)
    at ChildProcess.emit (events.js:198:13)
    at maybeClose (internal/child_process.js:982:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:259:5)
(node:54) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] test: `mocha test/index.test.js --timeout=5000`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/node/.npm/_logs/2020-09-28T05_41_14_227Z-debug.log

Can we make this test, Internal CI (functions/billing), not required to merge a PR?

@engelke
Copy link
Contributor

engelke commented Sep 28, 2020

Me too for PR #1985

@ace-n
Copy link
Contributor

ace-n commented Sep 28, 2020 via email

@ace-n
Copy link
Contributor

ace-n commented Sep 29, 2020

Update: looks like this test is outdated - it depends on client.hasScopes(), which is not defined on my local machine (and is causing the test to fail).

@JustinBeckwith I wasn't able to find a replacement in either of the google-auth or googleapis Node.js libraries. How should I replace this? 😕

@JustinBeckwith
Copy link
Contributor

I think you're looking for something like this:

const {GoogleAuth} = require('google-auth-library');
const auth = new GoogleAuth({
  scopes: [
    'https://www.googleapis.com/auth/cloud-billing',
    'https://www.googleapis.com/auth/cloud-platform',
  ],
});
google.options({auth});

@ace-n
Copy link
Contributor

ace-n commented Oct 7, 2020

Closing as duplicate of #1988 (which was fixed just now anyway).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: cloudfunctions Issues related to the Cloud Run functions API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. 🚨 This issue needs some love. samples Issues that are directly related to samples. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

7 participants