test(Spring CodeGen): Set up golden file tests for spring class composers

[emmileaf]

In this PR:

• Separates SpringPropertiesClassComposerTest from SpringAutoConfigClassComposerTest
• Replaces string comparisons with golden file testing

To update golden files:

mvn test -DupdateUnitGoldens -Dtest=SpringAutoConfigClassComposerTest
mvn test -DupdateUnitGoldens -Dtest=SpringPropertiesClassComposerTest
mvn test -DupdateUnitGoldens -Dtest=SpringComposerTest

Potential conflicts:

• fix(Spring CodeGen): Update missing and placeholder annotations #1045 should be merged first; resolve conflicts and update golden files in the PR once that happens
• Update golden files to include logger statements as well if feat(log): added log statements to gend autocfg #1053 is merged first

[emmileaf]

@zhumin8 Two open questions for this PR:

1. Right now the golden files for SpringComposerTest vs. the individual class composer tests only differ in the apache license. Would we want to keep these as separate files (vs. combine and reuse), given the additional testing for SpringComposer steps that we anticipate adding in the future?

2. I’ve split out SpringPropertiesClassComposerTest from SpringAutoConfigClassComposerTest, but noticed that the same chunk of setup code is shared and repeated. This setup seems to be replaceable by calling TestProtoLoader:

    this.context = TestProtoLoader.instance().parseShowcaseEcho();
    this.echoProtoService = this.context.services().get(0);

Was there a consideration for keeping the explicit setup steps here (perhaps for things that’d need customizing or flexibility in the future?) If not, I'll make the switch over in this PR.

[zhumin8]

Great work! this will make our work a lot easier when making changes to the Composer classes.

I only got one comment:
The setups for the three existing tests are exactly the same. Perhaps it makes sense to have an Abstract test class with the setups and each extend from it. This way if we ever want to test on more edge-cases in future, we are able to add the scenario to all test in sync.

On your questions above:

1. Changes to SpringComposer I have in mind now: 1) add package-info composer 2)extracting the @Generated("by gapic-generator-java") here instead of individual class composers. 3)perhaps add a new class composer for a shared property class.
   Either way, I would say combine and reuse is fine. Unless it causes trouble setting updates to golden files. Also, I don't have to strong a preference here, now that with golden files, they can be updated.
2. Great find! No particular reason. Your replacement is better. (I probably started out there testing around and lost track of the origin entirely)

[zhumin8]

just a note for future: we will need to carry this update logic over if we separate from gapic-generator-java.

[emmileaf]

@zhumin8 Thank you for the review! A quick summary of follow-up changes to the PR:

• Replaced the tests’ setup code (as discussed above)
• Moved tests and goldens into a composer subdirectory to align with package structure of the source code
• Tried changing the SpringComposer tests to reuse golden files from the individual class composer tests, but the extra setup felt more complicated than expected. Upon second thought, having a test use but not update the golden files (or having two different tests update the same ones) could get confusing, so I reverted back to separate files and just gave them better names. We can revisit this if needed later.

[zhumin8]

Changes looks good. Agreed on your reasoning on keeping separate golden files.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication