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(java): enable skipped tests for Native Image testing #1234

Merged
merged 25 commits into from
May 26, 2022

Conversation

mpeddada1
Copy link
Contributor

@mpeddada1 mpeddada1 commented Apr 21, 2022

Few changes to note:

  • Added gax-grpc to emulator module to bring in Native Image netty configurations
  • Ignoring tests that use Mockito for native image testing
  • Added resource configuration

@mpeddada1 mpeddada1 requested a review from a team as a code owner April 21, 2022 16:32
@product-auto-label product-auto-label bot added size: u Pull request is empty. api: bigtable Issues related to the googleapis/java-bigtable API. labels Apr 21, 2022
@mpeddada1 mpeddada1 requested a review from a team as a code owner April 21, 2022 16:34
@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. and removed size: u Pull request is empty. labels Apr 21, 2022
@mpeddada1
Copy link
Contributor Author

According to the logs:

 5 user-provided feature(s)
  - com.google.cloud.nativeimage.features.clients.CloudFunctionsFeature
  - com.google.cloud.nativeimage.features.clients.CloudSqlFeature
  - com.google.cloud.nativeimage.features.clients.SpannerFeature
  - com.oracle.svm.thirdparty.gson.GsonFeature
  - org.graalvm.junit.platform.JUnitPlatformFeature

The features in Gax are not getting picked up

@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: xs Pull request size is extra small. labels Apr 22, 2022
@generated-files-bot
Copy link

generated-files-bot bot commented Apr 22, 2022

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

  • .kokoro/build.sh
  • .kokoro/presubmit/graalvm-native.cfg

@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: xl Pull request size is extra large. labels Apr 22, 2022
@mpeddada1 mpeddada1 force-pushed the rename-tests branch 2 times, most recently from 7786ca0 to f4cf569 Compare April 22, 2022 00:28
@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. size: s Pull request size is small. and removed size: s Pull request size is small. size: xs Pull request size is extra small. labels Apr 22, 2022
@mpeddada1
Copy link
Contributor Author

Native Image Tests are now passing.

@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 22, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 22, 2022
@mpeddada1 mpeddada1 changed the title fix(java): rename test to be recognized by native profile fix(java): enable skipped tests for Native Image testing Apr 22, 2022
@mpeddada1
Copy link
Contributor Author

mpeddada1 commented May 18, 2022

@igorbernstein2 thanks again for your suggestions! any chance you could give this another review?

Copy link
Contributor

@igorbernstein2 igorbernstein2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!
thanks for putting this together

Copy link
Contributor

@igorbernstein2 igorbernstein2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, noticed one more thing: is there any way to remove the gax dependency?

@mpeddada1
Copy link
Contributor Author

Oops, noticed one more thing: is there any way to remove the gax dependency?

Mostly summarizing the offline discussion: using a provided scope only allows for the configurations to be included in the test jar. As mentioned here, the native image builder expects the configurations to be embedded into the project JAR through the META-INF/native-image/native-image.properties or META-INF/native-image/reflect-config.json and since com.google.cloud.bigtable.emulator.v2.Emulator references some netty classes, we still need these configurations in the main project JAR.

mpeddada1 and others added 6 commits May 20, 2022 18:03
…1255)

Source-Link: googleapis/synthtool@505ce5a
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:2567a120ce90fadb6201999b87d649d9f67459de28815ad239bce9ebfaa18a74

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
…cies to v2.12.0 (#1256)

[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [com.google.cloud:google-cloud-shared-dependencies](https://togithub.com/googleapis/java-shared-dependencies) | `2.11.0` -> `2.12.0` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-shared-dependencies/2.12.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-shared-dependencies/2.12.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-shared-dependencies/2.12.0/compatibility-slim/2.11.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-shared-dependencies/2.12.0/confidence-slim/2.11.0)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>googleapis/java-shared-dependencies</summary>

### [`v2.12.0`](https://togithub.com/googleapis/java-shared-dependencies/blob/HEAD/CHANGELOG.md#&#8203;2120-httpsgithubcomgoogleapisjava-shared-dependenciescomparev2110v2120-2022-05-19)

[Compare Source](https://togithub.com/googleapis/java-shared-dependencies/compare/v2.11.0...v2.12.0)

##### Features

-   add build scripts for native image testing in Java 17 ([#&#8203;1440](https://togithub.com/googleapis/java-shared-dependencies/issues/1440)) ([#&#8203;697](https://togithub.com/googleapis/java-shared-dependencies/issues/697)) ([f10ec4e](https://togithub.com/googleapis/java-shared-dependencies/commit/f10ec4e664d8fde868effe366b7182a5fad08dd0))

##### Dependencies

-   update gax.version to v2.18.1 ([#&#8203;695](https://togithub.com/googleapis/java-shared-dependencies/issues/695)) ([09bc61c](https://togithub.com/googleapis/java-shared-dependencies/commit/09bc61c9152a99bfe87554a07324f15ae6217d6e))
-   update google.core.version to v2.7.1 ([#&#8203;698](https://togithub.com/googleapis/java-shared-dependencies/issues/698)) ([43de259](https://togithub.com/googleapis/java-shared-dependencies/commit/43de2593f1a6e8fa5e34799364ab683246ddd449))

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-bigtable).
* build(fix): add in IT args for graalvm presubmit

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* Update owlbot.py

* it changes

* update graalvm config

* owlbot

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* Update graalvm-native.cfg

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 23, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 23, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 24, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 24, 2022
@mpeddada1 mpeddada1 requested a review from igorbernstein2 May 25, 2022 12:32
igorbernstein2 added a commit to igorbernstein2/java-bigtable that referenced this pull request May 26, 2022
…er with grpc helpers

Currently the emulator exists in a single artifact with optional deps. The reason for this is that bigtable-hbase needs the emulator w/o grpc. However this is causing issues in graalvm packaging in googleapis#1234. This PR makes this easier to manage: a -core artifact without dependencies that just wraps the golang binary that bigtable-hbase can use and a wrapper that has a hard dep on grpc & gax.

This is technically a breaking change but the emulator artifact is pre-GA an dis marked with BetaApi
igorbernstein2 added a commit to igorbernstein2/java-bigtable that referenced this pull request May 26, 2022
…er with grpc helpers

Currently the emulator exists in a single artifact with optional deps. The reason for this is
that bigtable-hbase needs the emulator w/o grpc. However this is causing issues in graalvm
packaging in googleapis#1234. This PR makes this easier to manage: a -core artifact without dependencies
that just wraps the golang binary that bigtable-hbase can use and a wrapper that has a hard dep
on grpc & gax.

This is technically a breaking change but the emulator artifact is pre-GA an is marked with BetaApi
gcf-merge-on-green bot pushed a commit that referenced this pull request May 26, 2022
…r with grpc helpers (#1264)

Currently the emulator exists in a single artifact with optional deps. The reason for this is that bigtable-hbase needs the emulator w/o grpc. However this is causing issues in graalvm packaging in #1234. This PR makes this easier to manage: a -core artifact without dependencies that just wraps the golang binary that bigtable-hbase can use and a wrapper that has a hard dep on grpc & gax.

This is technically a breaking change but the emulator artifact is pre-GA an is marked with BetaApi

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](https://github.com/googleapis/java-bigtable/issues/new/choose) 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 #<issue_number_goes_here> ☕️

If you write sample code, please follow the [samples format](
https://github.com/GoogleCloudPlatform/java-docs-samples/blob/main/SAMPLE_FORMAT.md).
@mpeddada1
Copy link
Contributor Author

@igorbernstein2 thanks for the refactoring! Tests are still green after rebasing.

Copy link
Contributor

@igorbernstein2 igorbernstein2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!
Thanks for all of your patience :)

@igorbernstein2 igorbernstein2 merged commit 560a501 into main May 26, 2022
@igorbernstein2 igorbernstein2 deleted the rename-tests branch May 26, 2022 20:47
gcf-merge-on-green bot pushed a commit that referenced this pull request May 31, 2022
🤖 I have created a release *beep* *boop*
---


## [2.8.0](v2.7.0...v2.8.0) (2022-05-27)


### Features

* split emulator into core without deps and a higher level wrapper with grpc helpers ([#1264](#1264)) ([6fdc2c1](6fdc2c1))


### Bug Fixes

* **java:** enable skipped tests for Native Image testing  ([#1234](#1234)) ([560a501](560a501))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants