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

fix(test): labels: should be an object, not array #315

Merged
merged 1 commit into from
Dec 21, 2018

Conversation

jkwlui
Copy link
Member

@jkwlui jkwlui commented Dec 20, 2018

🤦‍♂️

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 20, 2018
@jkwlui jkwlui changed the title fix(test): labels: should be an object, not arry fix(test): labels: should be an object, not array Dec 20, 2018
@jkwlui jkwlui force-pushed the fix-system-test-labels branch from bfb9a65 to a7e95e5 Compare December 20, 2018 23:56
@jkwlui jkwlui self-assigned this Dec 21, 2018
@jkwlui
Copy link
Member Author

jkwlui commented Dec 21, 2018

The remaining system-test failure is a little frustrating.. it is intermittent and I often have to run the system tests several times before getting it

      1) should error out for bad etags

  2 passing (5s)
  1 failing

  1) BigQuery
       BigQuery/Dataset
         should error out for bad etags:
     Uncaught 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.
      at emitDeprecationWarning (internal/process/promises.js:95:13)
      at emitWarning (internal/process/promises.js:88:3)
      at emitPromiseRejectionWarnings (internal/process/promises.js:120:9)
      at process._tickCallback (internal/process/next_tick.js:69:34)



(node:49173) UnhandledPromiseRejectionWarning: AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
+ expected - actual

- 400
+ 412

Upon logging the response bodies, I found that the server is returning 2 sets of response bodies for the same request:

body {
 "error": {
  "errors": [
   {
    "domain": "global",
    "reason": "conditionNotMet",
    "message": "Precondition Failed",
    "locationType": "header",
    "location": "If-Match"
   }
  ],
  "code": 412,
  "message": "Precondition Failed"
 }
}
body {
  "error": {
    "code": 400,
    "message": "Precondition check failed.",
    "errors": [
      {
        "message": "Precondition check failed.",
        "domain": "global",
        "reason": "failedPrecondition"
      }
    ],
    "status": "FAILED_PRECONDITION"
  }
}

But the weird thing is both responses give the 412 HTTP status code in the response header. When we parse the body, we override the status code returned to the client via ApiError with the error in the body, and that's why the test is failing.

@jkwlui
Copy link
Member Author

jkwlui commented Dec 21, 2018

Anyhow, it's probably unrelated to the test failure fixed by this PR, so let's merge this and I'll file an issue with the team.

@JustinBeckwith
Copy link
Contributor

Woah, that's wild. Filing a bug with the partner team feels like the right move. In the interim, I'm supportive of a retry or skip until the underlying API issue is resolved.

@JustinBeckwith JustinBeckwith merged commit 95ef658 into master Dec 21, 2018
@stephenplusplus stephenplusplus deleted the fix-system-test-labels branch February 26, 2019 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants