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

sql: divide udf logic tests into separate files #106729

Merged

Conversation

rharding6373
Copy link
Collaborator

Break up the UDF logic test into multiple files based on functionality in order to reduce risk of timeouts in long-running configurations.

Epic: none
Fixes: #106471

Release note: none

@rharding6373 rharding6373 requested review from mgartner and a team July 12, 2023 23:09
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rharding6373 rharding6373 force-pushed the 20230711_udf_logic_test_split_106471 branch 3 times, most recently from b87d7a7 to 3c2c089 Compare July 13, 2023 21:19
@rharding6373
Copy link
Collaborator Author

You can hold off on reviewing until I've added some of the newly proposed logic test style to the new test files (i.e., use subtest end, include pg error codes).

@rharding6373 rharding6373 marked this pull request as draft July 17, 2023 17:16
@mgartner
Copy link
Collaborator

You can hold off on reviewing until I've added some of the newly proposed logic test style to the new test files (i.e., use subtest end, include pg error codes).

👍 Nice! But if that becomes onerous, don't feel obligated to clean all that up now.

@rharding6373 rharding6373 force-pushed the 20230711_udf_logic_test_split_106471 branch 2 times, most recently from 67f7640 to 6208df4 Compare July 21, 2023 19:53
@rharding6373 rharding6373 marked this pull request as ready for review July 21, 2023 19:54
@rharding6373
Copy link
Collaborator Author

Ready for review now. I did not investigate the few questionable pg error codes yet, opting to leave them as a TODO for #107369.

Copy link
Collaborator

@mgartner mgartner left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! Looks like it was very tedious... :lgtm:

Reviewed 1 of 16 files at r1, 21 of 21 files at r2, 17 of 17 files at r3, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @rharding6373)

Break up the UDF logic test into multiple files based on functionality
in order to reduce risk of timeouts in long-running configurations.

Epic: none
Fixes: cockroachdb#106471

Release note: none
Used postgres where possible to generate the error codes. A few
questionable codes have been marked with TODOs to be addressed in future
work.

Epic: none
Informs: cockroachdb#107369

Release note: none
@rharding6373 rharding6373 force-pushed the 20230711_udf_logic_test_split_106471 branch from 6208df4 to b959f49 Compare July 21, 2023 22:19
@rharding6373
Copy link
Collaborator Author

TFTR!

I wrote a script [0] to try to make parts of the logic tests runnable in postgres to try to get the expected error codes, which was somewhat helpful but far from 100%. Needs some tweaking before it could be used to get error codes for all logic tests. Or maybe there's an easier way to run logic tests against postgres.

[0] https://gist.github.com/rharding6373/c6982084a2f810ebc8b754cd179a98bb

@rharding6373
Copy link
Collaborator Author

bors r+

@craig
Copy link
Contributor

craig bot commented Jul 22, 2023

Timed out.

@rharding6373
Copy link
Collaborator Author

Bors says that CI for this batch succeeded, but the overall state failed: https://bors.crdb.dev/batches/35879, which seems like a test infra problem.

Going to try one more time, since it seems like the UDF logic tests themselves finished well under the timeout.

bors r+

@craig
Copy link
Contributor

craig bot commented Jul 24, 2023

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pkg/sql/logictest/tests/fakedist/fakedist_test: TestLogic_udf failed
3 participants