feat(internal/surfer): add integration tests#4730
Conversation
…o integration-tests
…o integration-tests
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive golden file testing framework for the surfer generator, which is a great addition for ensuring stability and correctness. The changes also include removing internal Google links from comments, making the code more suitable for open source. My review focuses on the new testing framework in internal/surfer/surfer/golden_file_test.go. I've identified a few areas for improvement regarding error handling, the completeness of directory comparisons, and code clarity. Overall, this is a solid contribution that significantly improves the project's test coverage.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive integration test suite for the surfer tool using golden files, which is a great addition for ensuring the stability and correctness of the code generator. The test framework is well-structured, with good use of parallel tests, timeouts, and clear separation of test cases. I've identified a couple of areas for improvement in the test implementation to enhance its completeness and code quality. Specifically, the test verification should include all generated files, and a minor cleanup in a helper function signature would improve code clarity.
sarahheacock
left a comment
There was a problem hiding this comment.
I went a little crazy with the nits but love the test set up and how they work! This is better than what I had in mind
Add a golden file testing framework for surfer. This framework provides a consistent way to validate the final YAML output generated by `surfer. It ensures that changes to parsing and generation logic are tested against expected YAML outputs before being merged.
How the Integration Tests Work
The core testing logic is implemented in
golden_file_test.go. The test suite iterates over predefined scenarios (located ininternal/surfer/sufer/testdata/). For each scenario it:googleapisimports into a temporary structure.surfer generatecommand via the CLI using the defined test config and context.The expected YAMLs are divided into two distinct expectations, verified in subtests:
surfer: The current snapshot of whatsurferoutputs. (for CI)autogen: The intended, ideal state of output. This directory represents the target output we are aiming for in the future, mimicking the exact output of autogen (for development).Test Cases Covered
The test suite spans multiple edge scenarios and common API design patterns. Test cases tested in this framework include:
confirmation_promptcyclic_messages(currently skipped, tracked in Issue surfer: fix Infinite recursion in s parser for cyclic messages #4797)field_attributesfield_complex_typesfield_flag_namesfield_oneoffield_simple_typesfiltered_commandhelp_texthidden_commandhidden_featuremethod_asyncmethod_custommethod_minimal_listmethod_operationsmethod_output_formatmulti_service(currently skipped, tracked in Issue surfer: support Multiple Services in gcloud Generator #3291)multi_version_multi_track(currently skipped, tracked in Issue surfer: Implement Multi-Track Surface Generation #4530)regional_endpoints/global_onlyregional_endpoints/regional_requiredregional_endpoints/regional_supportedresource_multityperesource_non_standardresource_referenceresource_standardupdate_maskRunning and Updating Tests
surfergolden files: If you make an intentional parser/generator change and need to record the newly generatedsurferoutput as the expected golden state, run the tests with the-updateflag:go test ./internal/surfer/surfer -updateautogencomparison: By default, the tests only compare against thesurfergolden directory. To enable the comparison againstautogengolden files, use the-run-with-autogen-comparisonflag:go test ./internal/surfer/surfer -run-with-autogen-comparisonFixes # 4642