Addedrules_bazel/java_diff_test.bzl whereupdate_golden bazel rule is defined, including two parts: running JUnit test and overwritting goldens.
AddedSingleJUnitTestRunner which helps us to manually trigger the JUnit test, it takestest_class_name as arg and emits test result if any failure is detected.
AddedUtils.java intest/framework which helps to save the generated code from JUnit test to tmp file in folder$TEST_OUTPUT_HOME. System environment is set to interact with update golden bazel rules, the folder will get deleted once the bazel rules finish.
update_golden rule is called fromBUILD.bazel in dummy test, andjunit_runner_binary is defined for running the JUnit test when updating goldens.

How we use it (check out this branch and modify the dummy/goldens/*.golden)

> bazel test //...Tests have failures: [classWithHeader(com.google.api.generator.gapic.dummy.FileDiffInfraDummyTest): Differences found: --- golden+++ codegen@@ -17,3 +17,3 @@ package com.google.showcase.v1beta1.stub; -public class  {}+public class EchoStubSettings {}]//src/test/java/com/google/api/generator/gapic/dummy:FileDiffInfraDummyTest FAILED in 5.6s  /private/var/tmp/_bazel_xiaozhenliu/27d546162d4fdfcb8b9c40b4c5bf4971/execroot/com_google_api_codegen/bazel-out/darwin-fastbuild/testlogs/src/test/java/com/google/api/generator/gapic/dummy/FileDiffInfraDummyTest/test.log> bazel run //src/test/java/com/google/api/generator/gapic/dummy:FileDiffInfraDummyTest_update> bazel test //........Executed 53 out of 66 tests: 66 tests pass.

Next step would be:

Move the existing expected class string to file-diff infra, including
(a)gapic/composer/*Test.
(b)engine/astIntegrationTest
Update developer doc to include this instruction for updating goldens.

xiaozhenliu-gg5 added30 commits

September 8, 2020 17:56

add fileDiffUtils

42db152

add new dummy unit test

7c4b6a7

format

f42d269

add assertUtils

7224b5a

format

62ddbe6

move dummy test to separate folder

d9fbb0a

add compare strings method in diffUtils

0e63a94

format

076ef47

Merge branch 'dummy_test' of github.com:googleapis/gapic-generator-ja…

760b0a9

…va into java_diff_util

dummy test pass

25ce9ec

reformat

6f54239

merge

cf90743

add simple strings comparison

e3563b9

add java_diff_test bzl

2ff169e

group test framework helpers together

7cc8219

Merge branch 'java_diff_util' of github.com:googleapis/gapic-generato…

ec51871

…r-java into update_golden

run testRunner

2fce32e

format

306eb83

run single JUnit test and save output

53da6c4

add copy of golden

509746b

format

20b3b1f

working pipeline

3af8ba7

format

9e438ad

clean up

4c542c7

get codegen in local_tmp

6e08814

format

f6de85e

add golden path to bazel rules

b04ea6b

working pipeline

15e26e3

format

eb2f931

work!

278ade1

xiaozhenliu-gg5 added3 commits

September 19, 2020 14:57

feedback

f5f9ba9

clean

19d791c

comment

5ccd3ea

xiaozhenliu-gg5 requested a review frommiraleung

September 19, 2020 22:21

vam-google reviewed

Sep 22, 2020

View reviewed changes

Copy link

Contributor

vam-google left a comment•
edited
Loading

Choose a reason for hiding this comment

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

It looks good. I left quite a few comments, but most of them are minor and/or optional.

My only real concern is the general structure of the BUILD.bazel files (if we don't be careful here, we may end up in a very verbose layout, with many BUILD files everywhere and many targets being redeclared in each BUILD file, which will be very difficult to maintain).

dependencies.propertiesShow resolvedHide resolved

rules_bazel/java/java_diff_test.bzl OutdatedShow resolvedHide resolved

src/test/java/com/google/api/generator/gapic/dummy/BUILD.bazel OutdatedShow resolvedHide resolved

src/test/java/com/google/api/generator/test/framework/SingleJUnitTestRunner.java OutdatedShow resolvedHide resolved

src/test/java/com/google/api/generator/test/framework/Utils.java OutdatedShow resolvedHide resolved

src/test/java/com/google/api/generator/test/framework/Utils.javaShow resolvedHide resolved

src/test/java/com/google/api/generator/test/framework/Utils.java OutdatedShow resolvedHide resolved

src/test/java/com/google/api/generator/test/framework/Utils.javaShow resolvedHide resolved

xiaozhenliu-gg5 added3 commits

September 22, 2020 11:26

bazel rules feedbacj

a51f0ce

java code feedback

f230b09

move dependency

b51775d

miraleung reviewed

Sep 22, 2020

View reviewed changes

Copy link

Contributor

miraleung left a comment•
edited
Loading

Choose a reason for hiding this comment

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

It looks good. I left quite a few comments, but most of them are minor and/or optional.
My only real concern is the general structure of the BUILD.bazel files (if we don't be careful here, we may end up in a very verbose layout, with many BUILD files everywhere and many targets being redeclared in each BUILD file, which will be very difficult to maintain).

The motivation for the BUILD file structure in this repo is to keep individual repos self-contained (following google3 style), so the proliferation of files is fine.

Otherwise +1 to Vadym's comments below.

rules_bazel/java/java_diff_test.bzlShow resolvedHide resolved

src/test/java/com/google/api/generator/test/framework/Utils.javaShow resolvedHide resolved

xiaozhenliu-gg5 added3 commits

September 22, 2020 22:10

clean up

6f8d04b

merge master

f18c8cc

comment

d364be4

xiaozhenliu-gg5 requested review frommiraleung andvam-google

September 22, 2020 22:22

xiaozhenliu-gg5 added3 commits

September 23, 2020 17:52

move hamcrest dep back

c681dc1

Merge branch 'master' of github.com:googleapis/gapic-generator-java i…

1dc6ff3

…nto update_golden_bazel

add helpers in Utils

f91ee2c

xiaozhenliu-gg5 mentioned this pull request

Sep 23, 2020

[ggj][infra][4/5]feat: move the expected string in unit test to golden files#279

Merged

miraleung approved these changes

Sep 24, 2020

View reviewed changes

rules_bazel/java/java_diff_test.bzlShow resolvedHide resolved

xiaozhenliu-gg5 merged commit26611d7 intomaster

Sep 24, 2020

xiaozhenliu-gg5 deleted the update_golden_bazel branch

September 24, 2020 17:40

Copy link

Contributor

miraleung commentedOct 2, 2020

From a clean client, I tried running this update command, and got the error below, plus lots of warnings. Seems that this isn't working and requires each incremental target to be built first, whereas it should all just work withbazel run. I'll fix these issues too.

bazel run //src/test/java/com/google/api/generator/gapic/dummy:FileDiffInfraDummyTest_updateTarget //src/test/java/com/google/api/generator/gapic/dummy:FileDiffInfraDummyTest_update up-to-date:  bazel-bin/src/test/java/com/google/api/generator/gapic/dummy/FileDiffInfraDummyTest_update.shINFO: Elapsed time: 8.211s, Critical Path: 0.06sINFO: 0 processes.INFO: Build completed successfully, 4 total actionsINFO: Build completed successfully, 4 total actionsunzip:  cannot find or open bazel-out/k8-fastbuild/bin/src/test/java/com/google/api/generator/gapic/dummy/FileDiffInfraDummyTest_update_output.zip, bazel-out/k8-fastbuild/bin/src/test/java/com/google/api/generator/gapic/dummy/FileDiffInfraDummyTest_update_output.zip.zip or bazel-out/k8-fastbuild/bin/src/test/java/com/google/api/generator/gapic/dummy/FileDiffInfraDummyTest_update_output.zip.ZIP.

Copy link

Contributor

miraleung commentedOct 3, 2020

Another one I found: In a clean client, I get this error, even though it updates fine:

Error occurs when reading golden file

Copy link

Contributor

miraleung commentedOct 3, 2020

Fixed all of these in#363.

Copy link

ContributorAuthor

xiaozhenliu-gg5 commentedOct 3, 2020

This implementation was based on the prerequisites of users runningbazel test orbazel build first before updating any goldens files. The usage was described in the description, but it does not support directlybazel run testTarget_update for updating goldens before any test run.
Error occurs when reading golden file only happens when not finding/reading the goldens files, I did not have chance to repro that error, but thanks for the refactor to allow the direct usage of update_goldens.