Skip to content
This repository was archived by the owner on Feb 7, 2026. It is now read-only.

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

Merged
JustinBeckwith merged 1 commit intomasterfrom
fix-system-test-labels
Dec 21, 2018
Merged

fix(test): labels: should be an object, not array#315
JustinBeckwith merged 1 commit intomasterfrom
fix-system-test-labels

Conversation

@jkwlui
Copy link
Copy Markdown
Contributor

@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
Copy Markdown
Contributor 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
Copy Markdown
Contributor 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
Copy Markdown
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 subscribe to this conversation on GitHub. Already have an account? Sign in.

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