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.

Sign up for GitHub

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

Jump to bottom

Add ordering to to underspecified tests #8819

Merged
merged 6 commits into from
Sep 15, 2023
Merged

Add ordering to to underspecified tests #8819

merged 6 commits into from
Sep 15, 2023

Conversation

lnkuiper
Copy link
Contributor

@lnkuiper lnkuiper commented Sep 6, 2023

After merging #8475 and #8593, some queries' output order changed. Some of our tests were underspecified and relied on a specific output order. This mostly caused problems for our vector size = 2 tests.

I've added ordering to the tests so that they are deterministic again.

@lnkuiper
Copy link
Contributor Author

lnkuiper commented Sep 7, 2023

Failures are unrelated, I think this is ready to go

@github-actions github-actions bot marked this pull request as draft September 8, 2023 07:20
@lnkuiper
Copy link
Contributor Author

lnkuiper commented Sep 8, 2023

Someone fixed test/sql/window/test_mad_window.test and test/sql/function/list/lambdas/transform.test on main already it seems, conflicts are resolved now.

@lnkuiper lnkuiper marked this pull request as ready for review September 8, 2023 07:21
@github-actions github-actions bot marked this pull request as draft September 8, 2023 10:53
@lnkuiper lnkuiper marked this pull request as ready for review September 8, 2023 10:53
@github-actions github-actions bot marked this pull request as draft September 11, 2023 12:18
@lnkuiper lnkuiper marked this pull request as ready for review September 11, 2023 12:18
@lnkuiper
Copy link
Contributor Author

test/sql/parallelism/intraquery/test_parallel_nested_aggregates.test was also fixed on main, conflicts are resolved again.

@lnkuiper lnkuiper requested a review from pdet September 11, 2023 12:19
@lnkuiper
Copy link
Contributor Author

requested your review @pdet since we worked together on these vector size 2 tests

pdet
pdet approved these changes Sep 12, 2023
View reviewed changes
Copy link
Contributor

@pdet pdet left a comment

Choose a reason for hiding this comment

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

LGTM

@Mytherin Mytherin merged commit 6443a9e into duckdb:main Sep 15, 2023
49 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

@lnkuiper lnkuiper deleted the underspecified_tests branch November 24, 2023 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pdet pdet pdet approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@lnkuiper @Mytherin @pdet