Replacing gcd.sh as a local resource with a remote version #76

aozarov · 2015-05-27T00:18:43Z

No description provided.

gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/LocalGcdHelper.java

@@ -107,7 +122,14 @@ public void start() throws IOException, InterruptedException {
    File gcdFolder = gcdPath.toFile();
    gcdFolder.deleteOnExit();


coveralls · 2015-05-27T01:33:20Z

Coverage remained the same at 65.72% when pulling 7093b4b on aozarov:temp into 8881beb on GoogleCloudPlatform:master.

aozarov · 2015-05-27T02:46:15Z

PTAL

coveralls · 2015-05-27T03:56:08Z

Coverage remained the same at 65.72% when pulling 1616f63 on aozarov:temp into 8881beb on GoogleCloudPlatform:master.

coveralls · 2015-05-27T03:56:08Z

Coverage remained the same at 65.72% when pulling 1616f63 on aozarov:temp into 8881beb on GoogleCloudPlatform:master.

coveralls · 2015-05-27T03:56:08Z

Coverage remained the same at 65.72% when pulling 1616f63 on aozarov:temp into 8881beb on GoogleCloudPlatform:master.

coveralls · 2015-05-27T04:24:31Z

Coverage remained the same at 65.72% when pulling 1616f63 on aozarov:temp into 8881beb on GoogleCloudPlatform:master.

gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/LocalGcdHelper.java

@@ -52,8 +57,18 @@

  public static final String DEFAULT_PROJECT_ID = "projectid1";
  public static final int PORT = 8080;
-  private static final String GCD = "gcd-head";
-  private static final String GCD_LOC = '/' + GCD + ".zip";
+  private static final String GCD_VERSION = "gcd-v1beta2-rev1-2.1.2b";


jgeewax · 2015-05-27T15:51:14Z

I'm a tiny bit confused what the goal of this PR is.

Why can't we just say "If you want to run tests, first turn on a local datastore using gcd.sh" ? If anything this would ideall be integrated into the Cloud SDK such that you'll run $ gcloud datastore localserver start .... no?

Seems like a lot of work to download a special zip file when we could just put this in the documentation... (For reference, none of the other libraries have anything even remotely similar AFAIK.)

aozarov · 2015-05-27T15:58:11Z

These are unit-tests, which also run by Travis (though we could have handled Travis differently). I think it is much better user-experience avoiding a manual setup before running tests. The impact of downloading the zip would only happen for people that are running the tests and the zip will be cached and will not require downloads in subsequent tests/runs. I think this one time (or once in a while when cache cleared) download is negligible compared to the tests runtime.

jgeewax · 2015-05-27T16:03:12Z

I think we agree that there should be no manual set up before unit tests.

I'm adding that I think unit tests should run entirely in-process rather than using a separate server against which to execute the tests.

AFAIK, unit tests are supposed to check that we're making the right calls against the external APIs (in this case, the GCD API). They aren't supposed to check that the response is correct.

To put this as an example, I'd expect that we check that we populate the protobuf correctly in unit tests, but our test ends when we check that a mock in-process API server received that protobuf, and validated that it was as we expected. I'd expect an integration test to verify that the response is what we expected.

aozarov · 2015-05-27T16:15:08Z

I agree that external process is not ideal for unit-tests but we are trying to make that transparent using the LocalGcdHelper utility and that will changes in the future once the gcd tool supports it (it was
considered before but direction currently is not known). Most AE unit-tests are using a similar approach when using LocalServiceTestHelper (which is basically the only way to write/run unit-tests that are using any of AE services). This tool could also serve application's code that are using gcloud-java-datastore.

If you provide a method "M" that gets input "I" call service "S", process the service response and return result "R" you probably want to make sure that the assumption "M" makes in its code about what "S" needs and what "S" returns are correct. If you mock "S" response you only validate half of your code (assuming you are not returning "S"'s result directly).

Also, per your example. How do you know that the protobuf is indeed populated correctly based on the service needs?

jgeewax · 2015-05-27T16:23:37Z

I'm saying "in unit tests, make the assumption". We don't even know if there is potentially a bug in the local-server -- and many services don't have these, and many future ones don't plan to offer these.

Integration tests are there to double check that the assumptions are correct.

aozarov · 2015-05-27T16:37:24Z

But if you already have a local-servee why don't you use it? As said the cost of getting it is negligible (please try it) and benefits are high.

coveralls · 2015-05-27T18:33:47Z

Coverage remained the same at 65.72% when pulling 8352ecd on aozarov:temp into 8881beb on GoogleCloudPlatform:master.

coveralls · 2015-05-27T20:43:34Z

Coverage remained the same at 65.72% when pulling 8352ecd on aozarov:temp into 8881beb on GoogleCloudPlatform:master.

ludoch · 2015-05-27T20:54:55Z

LGTM

Replacing gcd.sh as a local resource with a remote version

… to v1.0.1 (#76) * chore(deps): update dependency com.google.cloud:google-cloud-dataplex to v1.0.1 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

…0.0 (#76) [![Mend 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-storage](https://togithub.com/googleapis/java-storage) | `2.7.0` -> `2.10.0` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-storage/2.10.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-storage/2.10.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-storage/2.10.0/compatibility-slim/2.7.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-storage/2.10.0/confidence-slim/2.7.0)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. ⚠ **Warning**: custom changes will be lost. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-optimization).

🤖 I have created a release *beep* *boop* --- ## [1.1.3](googleapis/java-optimization@v1.1.2...v1.1.3) (2022-08-02) ### Dependencies * update dependency com.google.cloud:google-cloud-shared-dependencies to v3.0.1 ([#78](googleapis/java-optimization#78)) ([5fb0e92](googleapis/java-optimization@5fb0e92)) * update dependency com.google.cloud:google-cloud-storage to v2.10.0 ([#76](googleapis/java-optimization#76)) ([2168a43](googleapis/java-optimization@2168a43)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- ## [0.5.2](googleapis/java-video-live-stream@v0.5.1...v0.5.2) (2022-08-12) ### Dependencies * update dependency com.google.cloud:google-cloud-shared-dependencies to v3 ([#76](googleapis/java-video-live-stream#76)) ([59e99ce](googleapis/java-video-live-stream@59e99ce)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

…1575) (#76) Source-Link: googleapis/synthtool@2e9ac19 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:8175681a918181d306d9c370d3262f16b4c724cc73d74111b7d42fc985ca7f93

🤖 I have created a release *beep* *boop* --- ## [0.3.4](googleapis/java-video-stitcher@v0.3.3...v0.3.4) (2022-09-15) ### Dependencies * Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.0.3 ([#75](googleapis/java-video-stitcher#75)) ([05019e3](googleapis/java-video-stitcher@05019e3)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- ### Updating meta-information for bleeding-edge SNAPSHOT release. --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [keyring](https://togithub.com/jaraco/keyring) | `==23.4.1` -> `==23.9.3` | [![age](https://badges.renovateapi.com/packages/pypi/keyring/23.9.3/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/pypi/keyring/23.9.3/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/pypi/keyring/23.9.3/compatibility-slim/23.4.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/pypi/keyring/23.9.3/confidence-slim/23.4.1)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. ⚠ **Warning**: custom changes will be lost. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-bigquery-data-exchange).

- [ ] Regenerate this pull request now. PiperOrigin-RevId: 477955264 Source-Link: https://togithub.com/googleapis/googleapis/commit/a724450af76d0001f23602684c49cd6a4b3a5654 Source-Link: https://togithub.com/googleapis/googleapis-gen/commit/4abcbcaec855e74a0b22a4988cf9e0eb61a83094 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNGFiY2JjYWVjODU1ZTc0YTBiMjJhNDk4OGNmOWUwZWI2MWE4MzA5NCJ9

#76) - [ ] Regenerate this pull request now. PiperOrigin-RevId: 485623855 Source-Link: https://togithub.com/googleapis/googleapis/commit/807125e7953e32535710924fe4bf0362ac93754d Source-Link: https://togithub.com/googleapis/googleapis-gen/commit/c4ddd6d651da69658a4762c5eafff5cd8f0e25c5 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYzRkZGQ2ZDY1MWRhNjk2NThhNDc2MmM1ZWFmZmY1Y2Q4ZjBlMjVjNSJ9

aozarov added 2 commits May 26, 2015 16:33

initial work on removing gcd as a resource

7385fb5

continue work on replacing local gcd.sh with a remote version

7093b4b

aozarov assigned ludoch May 27, 2015

ludoch reviewed May 27, 2015
View reviewed changes

aozarov added 2 commits May 26, 2015 19:27

add some comments describing how the LocalGcdHelper works

fc73469

display stderr of gcd create

1616f63

jgeewax reviewed May 27, 2015
View reviewed changes

aozarov mentioned this pull request May 27, 2015

Remove binary blobs from history? #11

Closed

aozarov added 2 commits May 27, 2015 09:48

replace GCD_VERSION with GCD

7f03b07

add a checksum verification

8352ecd

ludoch added a commit that referenced this pull request May 27, 2015

Merge pull request #76 from aozarov/temp

2fef9bf

Replacing gcd.sh as a local resource with a remote version

ludoch merged commit 2fef9bf into googleapis:master May 27, 2015

ludoch added a commit that referenced this pull request Jul 1, 2015

Merge pull request #76 from aozarov/temp

bf9e7e9

Replacing gcd.sh as a local resource with a remote version

ludoch added a commit that referenced this pull request Jul 7, 2015

Merge pull request #76 from aozarov/temp

c64b3d7

Replacing gcd.sh as a local resource with a remote version

theacodes unassigned ludoch Sep 28, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replacing gcd.sh as a local resource with a remote version #76

Replacing gcd.sh as a local resource with a remote version #76

aozarov commented May 27, 2015

This comment was marked as spam.

This comment was marked as spam.

coveralls commented May 27, 2015

aozarov commented May 27, 2015

coveralls commented May 27, 2015

coveralls commented May 27, 2015

coveralls commented May 27, 2015

coveralls commented May 27, 2015

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

jgeewax commented May 27, 2015

aozarov commented May 27, 2015

jgeewax commented May 27, 2015

aozarov commented May 27, 2015

jgeewax commented May 27, 2015

aozarov commented May 27, 2015

coveralls commented May 27, 2015

coveralls commented May 27, 2015

ludoch commented May 27, 2015

		@@ -107,7 +122,14 @@ public void start() throws IOException, InterruptedException {
		File gcdFolder = gcdPath.toFile();
		gcdFolder.deleteOnExit();

Replacing gcd.sh as a local resource with a remote version #76

Replacing gcd.sh as a local resource with a remote version #76

Conversation

aozarov commented May 27, 2015

This comment was marked as spam.

This comment was marked as spam.

coveralls commented May 27, 2015

aozarov commented May 27, 2015

coveralls commented May 27, 2015

coveralls commented May 27, 2015

coveralls commented May 27, 2015

coveralls commented May 27, 2015

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

jgeewax commented May 27, 2015

aozarov commented May 27, 2015

jgeewax commented May 27, 2015

aozarov commented May 27, 2015

jgeewax commented May 27, 2015

aozarov commented May 27, 2015

coveralls commented May 27, 2015

coveralls commented May 27, 2015

ludoch commented May 27, 2015