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

test: Replace deprecated method with non-deprecated one #279

Merged

Conversation

irvifa
Copy link
Contributor

@irvifa irvifa commented Jul 4, 2020

Since we have several deprecated method in the ITPubSubTest,
I think we can begin to migrate it to non-deprecated method one.

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #278 ☕️

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 4, 2020
@irvifa irvifa force-pushed the test/replace-deprecated-method branch from 591e0b3 to e5bd57b Compare July 4, 2020 09:06
@codecov
Copy link

codecov bot commented Jul 4, 2020

Codecov Report

Merging #279 into master will increase coverage by 0.13%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #279      +/-   ##
============================================
+ Coverage     79.05%   79.18%   +0.13%     
- Complexity      312      317       +5     
============================================
  Files            21       21              
  Lines          2841     2888      +47     
  Branches        153      155       +2     
============================================
+ Hits           2246     2287      +41     
- Misses          533      537       +4     
- Partials         62       64       +2     
Impacted Files Coverage Δ Complexity Δ
...com/google/cloud/pubsub/v1/TopicAdminSettings.java 19.60% <0.00%> (-0.81%) 5.00% <0.00%> (ø%)
...com/google/cloud/pubsub/v1/stub/PublisherStub.java 6.25% <0.00%> (-0.42%) 1.00% <0.00%> (ø%)
.../com/google/cloud/pubsub/v1/MessageDispatcher.java 84.87% <0.00%> (-0.28%) 26.00% <0.00%> (+1.00%) ⬇️
...in/java/com/google/cloud/pubsub/v1/Subscriber.java 80.00% <0.00%> (ø) 22.00% <0.00%> (ø%)
...le/cloud/pubsub/v1/stub/PublisherStubSettings.java 83.56% <0.00%> (+0.29%) 21.00% <0.00%> (+1.00%)
...google/cloud/pubsub/v1/stub/GrpcPublisherStub.java 96.09% <0.00%> (+0.33%) 19.00% <0.00%> (+1.00%)
...a/com/google/cloud/pubsub/v1/TopicAdminClient.java 58.95% <0.00%> (+0.48%) 34.00% <0.00%> (+2.00%)
...cloud/pubsub/v1/StreamingSubscriberConnection.java 65.18% <0.00%> (+0.79%) 9.00% <0.00%> (ø%)

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 270bd94...3a61089. Read the comment docs.

@irvifa irvifa force-pushed the test/replace-deprecated-method branch 5 times, most recently from bbaebb3 to 2e68b3a Compare July 4, 2020 09:45
@hannahrogers-google hannahrogers-google added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 6, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 6, 2020
@irvifa irvifa force-pushed the test/replace-deprecated-method branch from 2e68b3a to 8aa4425 Compare July 7, 2020 09:30
@irvifa
Copy link
Contributor Author

irvifa commented Jul 7, 2020

HI @hannahrogers-google I already amend review suggestions on the latest commit.

@irvifa irvifa force-pushed the test/replace-deprecated-method branch from 8aa4425 to aa147bc Compare July 7, 2020 09:32
@hannahrogers-google hannahrogers-google added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 7, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 7, 2020
@irvifa irvifa force-pushed the test/replace-deprecated-method branch from aa147bc to cf25beb Compare July 9, 2020 08:17
@irvifa
Copy link
Contributor Author

irvifa commented Jul 9, 2020

@hannahrogers-google I fixed the method by adding the bindings:

policy.toBuilder().addBindings(binding).build()

@hannahrogers-google hannahrogers-google added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 9, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 9, 2020
@hannahrogers-google
Copy link
Contributor

Thanks @irvifa, the PR looks good to me. A couple of tests seem to be stuck in the 'Expected — Waiting for status to be reported' state. Can you try pushing a fresh commit to hopefully resolve this issue?

Since we have several deprecated method in the ITPubSubTest,
I think we can begin to migrate it to non-deprecated method one.
@irvifa irvifa force-pushed the test/replace-deprecated-method branch from cf25beb to 3a61089 Compare July 9, 2020 23:55
@irvifa
Copy link
Contributor Author

irvifa commented Jul 9, 2020

@hannahrogers-google yep, I tried to push it again, hopefully it won't got stuck this time. However it will need another kokoro test again.

@hannahrogers-google hannahrogers-google added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 10, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 10, 2020
@hannahrogers-google
Copy link
Contributor

@hannahrogers-google yep, I tried to push it again, hopefully it won't got stuck this time. However it will need another kokoro test again.

thanks, looks like they are all running this time :) I will merge when the tests complete

@hannahrogers-google hannahrogers-google merged commit 754b14c into googleapis:master Jul 10, 2020
@irvifa irvifa deleted the test/replace-deprecated-method branch July 10, 2020 00:20
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.

test: Replace deprecated method with non-deprecated one
4 participants