Fix system tests. #190

stephenplusplus · 2018-05-02T21:46:21Z

Do not merge: #190 (comment)

CI has been failing: https://circleci.com/gh/googleapis/nodejs-storage/4098.

system-test/storage.js

@@ -1282,7 +1275,7 @@ describe('storage', function() {
              }

              // Re-create the file. The tests need it.
-              file.save('newcontent', done);
+              file.save('newcontent', options, done);


alexander-fenster

What about other 2/5 now? :)

system-test/storage.js

@@ -1228,13 +1228,6 @@ describe('storage', function() {
          })
        );

-        it(


system-test/storage.js

@@ -1385,14 +1378,6 @@ describe('storage', function() {
          })
        );

-        it(


stephenplusplus · 2018-05-02T21:48:20Z

@frankyn Our system tests around requester pays started failing. I left comments on the diff-- could you take a look? Thanks!

codecov · 2018-05-03T00:02:48Z

Codecov Report

Merging #190 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master   #190   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           7      7           
  Lines        1104   1104           
=====================================
  Hits         1104   1104

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 700c780...b5acc01. Read the comment docs.

frankyn · 2018-05-03T20:06:55Z

acking* I'm at a conference this week and will have delayed responses.

googlebot · 2018-05-03T20:07:43Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

googlebot · 2018-05-07T14:29:52Z

☹️ Sorry, but only Googlers may change the label cla: yes.

stephenplusplus · 2018-05-09T17:36:45Z

@frankyn any availability to give this a quick look?

frankyn · 2018-05-09T17:55:14Z

Hey @stephenplusplus, taking a look now.

frankyn · 2018-05-09T18:09:36Z

Pinged GCS team for more context, resumable upload should be required to define user project unless I misunderstood. The testIamPermission is not as clear. I'll ping when I have an update.

googlebot · 2018-05-09T18:29:25Z

CLAs look good, thanks!

googlebot · 2018-05-09T18:29:27Z

☹️ Sorry, but only Googlers may change the label cla: yes.

stephenplusplus · 2018-05-09T18:31:22Z

Thanks @frankyn!

googlebot · 2018-05-09T18:45:49Z

☹️ Sorry, but only Googlers may change the label cla: yes.

googlebot · 2018-05-09T19:13:29Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

lukesneeringer · 2018-05-09T19:14:06Z

@googlebot The reason for overriding the CLA status is because you are wrong and should feel bad. :-)

We really need to get @stephenplusplus whitelisted on this thing...someday...

JustinBeckwith · 2018-05-10T03:15:45Z

gentle ping what's the status on this one folks?

frankyn · 2018-05-11T20:21:16Z

@JustinBeckwith we are pending storage service bug on requester pays enabled buckets. bug:79524739 for more context. The gcs team is looking at it, it's acked as a potential bug at this time. We can skip tests for these two cases for now in order to move forward with this PR and we can revisit when there is a solution. Aka, bug is fixed.

Restore test, but skip it.

googlebot · 2018-05-11T20:39:40Z

CLAs look good, thanks!

stephenplusplus · 2018-05-11T20:41:09Z

Thanks for your help, @frankyn!

I've left the test in, but our test runner will skip it. There's also a @TODO with more information about why it's skipped.

Once tests pass, I'll merge this in.

stephenplusplus requested review from callmehiphop and alexander-fenster May 2, 2018 21:46

googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 2, 2018

ghost assigned stephenplusplus May 2, 2018

stephenplusplus commented May 2, 2018

View reviewed changes

system-test/storage.js

@@ -1282,7 +1275,7 @@ describe('storage', function() {

}

// Re-create the file. The tests need it.

file.save('newcontent', done);

file.save('newcontent', options, done);

This comment was marked as spam.

Sign in to view

alexander-fenster approved these changes May 2, 2018

View reviewed changes

stephenplusplus commented May 2, 2018

View reviewed changes

system-test/storage.js

@@ -1385,14 +1378,6 @@ describe('storage', function() {

})

);

it(

This comment was marked as spam.

Sign in to view

callmehiphop approved these changes May 3, 2018

View reviewed changes

ghost assigned callmehiphop May 3, 2018

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels May 3, 2018

stephenplusplus mentioned this pull request May 7, 2018

Fallback to non-resumable not working when $HOME is not writable #181

Closed

stephenplusplus added the cla: yes This human has signed the Contributor License Agreement. label May 7, 2018

googlebot removed the cla: yes This human has signed the Contributor License Agreement. label May 7, 2018

stephenplusplus added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 7, 2018

ghost added the cla: yes This human has signed the Contributor License Agreement. label May 9, 2018

googlebot removed the cla: yes This human has signed the Contributor License Agreement. label May 9, 2018

stephenplusplus added 2 commits May 9, 2018 14:25

Fix system tests.

190b509

Test resumable uploads more thoroughly

1e4fc67

stephenplusplus force-pushed the spp--ci-fixes branch from 0b4c78d to 1e4fc67 Compare May 9, 2018 18:29

ghost added the cla: yes This human has signed the Contributor License Agreement. label May 9, 2018

googlebot removed the cla: no This human has *not* signed the Contributor License Agreement. label May 9, 2018

googlebot removed the cla: yes This human has signed the Contributor License Agreement. label May 9, 2018

lint

737ee8d

ghost added the cla: yes This human has signed the Contributor License Agreement. label May 9, 2018

googlebot removed the cla: yes This human has signed the Contributor License Agreement. label May 9, 2018

lukesneeringer added the cla: yes This human has signed the Contributor License Agreement. label May 9, 2018

Update storage.js

b5acc01

Restore test, but skip it.

stephenplusplus removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 11, 2018

stephenplusplus merged commit 496026e into googleapis:master May 11, 2018

ghost removed the cla: yes This human has signed the Contributor License Agreement. label May 11, 2018

stephenplusplus deleted the spp--ci-fixes branch May 11, 2018 20:46

stephenplusplus mentioned this pull request Jun 22, 2018

system-tests: re-enable iam#testPermissions requesterPays test. #251

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix system tests. #190

Fix system tests. #190

stephenplusplus commented May 2, 2018 •

edited

Loading

This comment was marked as spam.

alexander-fenster left a comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

stephenplusplus commented May 2, 2018

codecov bot commented May 3, 2018 •

edited

Loading

frankyn commented May 3, 2018

googlebot commented May 3, 2018

googlebot commented May 7, 2018

stephenplusplus commented May 9, 2018

frankyn commented May 9, 2018

frankyn commented May 9, 2018

googlebot commented May 9, 2018

googlebot commented May 9, 2018

stephenplusplus commented May 9, 2018

googlebot commented May 9, 2018

googlebot commented May 9, 2018

lukesneeringer commented May 9, 2018 •

edited

Loading

JustinBeckwith commented May 10, 2018

frankyn commented May 11, 2018 •

edited

Loading

googlebot commented May 11, 2018

stephenplusplus commented May 11, 2018

@@ @@ -1228,13 +1228,6 @@ describe('storage', function() { @@
                         })
                       );
-                      it(

@@ @@ -1385,14 +1378,6 @@ describe('storage', function() { @@
                         })
                       );
-                      it(

Fix system tests. #190

Fix system tests. #190

Conversation

stephenplusplus commented May 2, 2018 • edited Loading

Do not merge: #190 (comment)

This comment was marked as spam.

alexander-fenster left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

stephenplusplus commented May 2, 2018

codecov bot commented May 3, 2018 • edited Loading

Codecov Report

frankyn commented May 3, 2018

googlebot commented May 3, 2018

googlebot commented May 7, 2018

stephenplusplus commented May 9, 2018

frankyn commented May 9, 2018

frankyn commented May 9, 2018

googlebot commented May 9, 2018

googlebot commented May 9, 2018

stephenplusplus commented May 9, 2018

googlebot commented May 9, 2018

googlebot commented May 9, 2018

lukesneeringer commented May 9, 2018 • edited Loading

JustinBeckwith commented May 10, 2018

frankyn commented May 11, 2018 • edited Loading

googlebot commented May 11, 2018

stephenplusplus commented May 11, 2018

stephenplusplus commented May 2, 2018 •

edited

Loading

codecov bot commented May 3, 2018 •

edited

Loading

lukesneeringer commented May 9, 2018 •

edited

Loading

frankyn commented May 11, 2018 •

edited

Loading