Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci(spanner): improve performance of samples tests #3558

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

sakthivelmanii
Copy link
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> ☕️

If you write sample code, please follow the samples format.

@sakthivelmanii sakthivelmanii requested review from a team as code owners December 16, 2024 09:42
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: spanner Issues related to the googleapis/java-spanner API. samples Issues that are directly related to samples. labels Dec 16, 2024
@sakthivelmanii sakthivelmanii force-pushed the optimize_sample_test_performance branch 3 times, most recently from 4426b9d to 63189a3 Compare December 16, 2024 10:08
@sakthivelmanii sakthivelmanii force-pushed the optimize_sample_test_performance branch from 63189a3 to 2782d6b Compare December 16, 2024 10:11
Comment on lines +150 to +151
<forkCount>10</forkCount>
<reuseForks>false</reuseForks>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this does anything at the moment, as there are no integration tests defined in this project. Are we planning on adding tests here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is. Currently we are runnings tests three times in all three modules.

Comment on lines +149 to +150
<forkCount>10</forkCount>
<reuseForks>false</reuseForks>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as for install-without-bom: There are no tests in this project at the moment, so I don't think this does anything

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is. Currently we are runnings tests three times in all three modules.

Comment on lines +192 to +194
<excludes>
<exclude>**/SpannerSampleIT.java</exclude>
</excludes>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand that this one is slow, but do we really want to skip it all-together? And/or will we have some other nightly job that overrides this configuration and does include this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am introducing a new job to run this test alone in parallel so that it can be completed in an hour.

@sakthivelmanii sakthivelmanii merged commit 557b214 into main Dec 16, 2024
34 checks passed
@sakthivelmanii sakthivelmanii deleted the optimize_sample_test_performance branch December 16, 2024 13:46
surbhigarg92 pushed a commit to surbhigarg92/java-spanner that referenced this pull request Dec 26, 2024
gagangupt16 pushed a commit to gagangupt16/java-spanner that referenced this pull request Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/java-spanner API. samples Issues that are directly related to samples. size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants