test(spanner): additional tests for SelectAll()

[olavloite]

Add some additional test cases for SelectAll() to verify that the behavior remains the same if #13316 is merged.

[gemini-code-assist]

Summary of Changes

Hello @olavloite, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the test suite for the SelectAll() function within the Spanner client library. The primary goal is to ensure the function's robustness and predictable behavior when mapping query results to complex Go struct types that include nested or embedded structs. These new tests are crucial for maintaining stability, especially in light of potential future changes to the library's data mapping logic.

Highlights

• New Test Cases for SelectAll(): Introduced additional test cases for the SelectAll() function to specifically verify its behavior when dealing with nested Go structs, both named and embedded.
• Nested Struct Handling: Added test scenarios to confirm that SelectAll() correctly handles or reports errors/panics for complex struct types, including Person with an Address field and PersonEmbedded with an embedded Address struct.
• Panic Testing: Implemented a mechanism to test for expected panics within the TestSelectAll function, ensuring that certain invalid operations with nested embedded structs trigger the anticipated runtime errors.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request adds test cases for SelectAll to cover scenarios with nested and embedded structs, ensuring current error and panic behaviors are captured. The changes are logical and the new tests are valuable. I've added a couple of suggestions to improve the maintainability and clarity of the new test code by reducing duplication and making the test case for panics more robust.

[gemini-code-assist]

The mock iterator creation logic is duplicated between the failure: nested named structs and failure: nested unnamed structs test cases. To improve maintainability and reduce code duplication, you could extract this logic into a helper function that returns a new mock iterator on each call. This also allows you to de-duplicate the []*sppb.StructType_Field slice which is identical for both rows in the iterator.

For example, you could define a helper function:

func newPersonMockIterator() *mockIterator {
	personFields := []*sppb.StructType_Field{
		{Name: "Name", Type: stringType()},
		{Name: "Street", Type: stringType()},
		{Name: "ZipCode", Type: stringType()},
		{Name: "City", Type: stringType()},
		{Name: "BirthDate", Type: dateType()},
	}
	return newMockIterator(
		&Row{
			personFields,
			[]*proto3.Value{
				stringProto("Name1"), stringProto("Street1"), stringProto("ZipCode1"), stringProto("City1"), dateProto(civil.Date{Year: 2000, Month: 11, Day: 14}),
			},
		},
		&Row{
			personFields,
			[]*proto3.Value{
				stringProto("Name2"), stringProto("Street2"), stringProto("ZipCode2"), stringProto("City2"), dateProto(civil.Date{Year: 2001, Month: 11, Day: 14}),
			},
		},
		iterator.Done,
	)
}

And then call it in your test cases:

//...
{
	name: "failure: nested named structs",
	args: args{
		destination: &[]*Person{},
		mock:        newPersonMockIterator(),
	},
	want:    &[]*Person{},
	wantErr: true,
},
{
	name: "failure: nested unnamed structs",
	args: args{
		destination: &[]*PersonEmbedded{},
		mock:        newPersonMockIterator(),
	},
	want: &[]*PersonEmbedded{},
	wantPanic: true,
},
//...

[gemini-code-assist]

For clarity and to prevent an additional failure message if the code stops panicking in the future, it's good practice to specify the expected state of destination even in a panic test. Since SelectAll shouldn't modify the destination if it panics (or errors out before completion), the expected value want should be the same as the initial destination.

			want:      &[]*PersonEmbedded{},
			wantPanic: true,