Skip to content

Conversation

@diegomarquezp
Copy link
Contributor

@diegomarquezp diegomarquezp commented Jul 5, 2024

to be merged once gapic-genreator-java is updated to >2.42.0

Adapts the toString tests of *SettingsTest given the new internal variables using java.time in gax (introduced by googleapis/sdk-platform-java#1872)

Confirmed it works by using gax 2.51.1-SNAPSHOT
image

BEGIN_COMMIT_OVERRIDE
fix: adapt toString tests to introduction of java.time in gax
deps: update shared dependencies
END_COMMIT_OVERRIDE

@diegomarquezp diegomarquezp added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 5, 2024
@diegomarquezp diegomarquezp requested review from a team as code owners July 5, 2024 00:22
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: bigtable Issues related to the googleapis/java-bigtable API. labels Jul 5, 2024
diegomarquezp added a commit to googleapis/sdk-platform-java that referenced this pull request Jul 8, 2024
…all (#3016)

Fixes #3015
Fixes #3014

Gax tracing internally works with `attemptFailedDuration`, which
defaults to a no-op. Downstream libraries use `attemptFailed`, which has
a custom behavior. What happens when an attempt-failed event occurs is
that `attemptFailedDuration` is called instead (in favor of using
java.time methods internally). This fix makes `attemptFailedDuration`'s
behavior to delegate the logic to `attemptFailed`.

The downstreams will keep failing because the repos haven't got adapted
to the new change in gax. See the follow ups below.

### Fixes in `java-spanner`

![image](https://github.com/googleapis/sdk-platform-java/assets/22083784/85e50341-fe8b-46c8-9743-8de1ca300058)

### Fixes in `java-bigtable`

![image](https://github.com/googleapis/sdk-platform-java/assets/22083784/026af401-1607-4465-abd5-1ee5ddc353d0)


### Follow ups in `java-bigtable`
More failures in java-bigtable to be addressed in that repo:
```
Error:    BigtableTableAdminSettingsTest.testToString:175 expected to contain: totalTimeout=PT13H32M
but was            : BigtableTableAdminSettings{projectId=our-project-85, instanceId=our-instance-06, 
...
```

Fixed in googleapis/java-bigtable#2274

### Follow ups in `java-spanner`
```
Error:  Failures: 
Error:    CompositeTracerTest.testMethodsOverrideMetricsTracer:238 Method not found in compositeTracerMethods: public void com.google.api.gax.tracing.MetricsTracer.attemptFailedDuration(java.lang.Throwable,java.time.Duration)
```
Fixed in googleapis/java-spanner#3200
@lqiu96 lqiu96 requested a review from a team as a code owner August 5, 2024 14:46
@generated-files-bot
Copy link

Warning: This pull request is touching the following templated files:

  • .kokoro/presubmit/graalvm-native.cfg

@lqiu96 lqiu96 added the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 5, 2024
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 5, 2024
@lqiu96 lqiu96 added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 5, 2024
@lqiu96 lqiu96 mentioned this pull request Aug 5, 2024
1 task
@lqiu96
Copy link
Member

lqiu96 commented Aug 5, 2024

@igorbernstein2 I merged #2274 into this PR. I believe this PR will need the latest shared-deps to pass. This fix should resolve the failing unit tests in the other PRs.

@igorbernstein2 igorbernstein2 merged commit 93f66a7 into main Aug 6, 2024
@igorbernstein2 igorbernstein2 deleted the fix-javatime-tests branch August 6, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the googleapis/java-bigtable API. size: s Pull request size is small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants