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

Test and fix debug mode #2481

Merged
merged 18 commits into from
Nov 12, 2024
Merged

Test and fix debug mode #2481

merged 18 commits into from
Nov 12, 2024

Conversation

ADBond
Copy link
Contributor

@ADBond ADBond commented Oct 22, 2024

Some tests of various bits of functionality with debug_mode switched on. Tests are structured so that we can check that issues specifically arise only when debug_mode is on - if the test fails when it is off, a different error will be flagged, to help isolate things. These can be expanded to help diagnose issues, but just as an initial starting point.

This will also include fixes for the failing tests, which will be opened as separate PRs into this branch:

Partial work towards #2429. This tackles 'does stuff work when debug_mode is switched on, but doesn't address 'is debug_mode doing what we think it is'.

@ADBond ADBond added bug Something isn't working testing debug_mode labels Oct 22, 2024
@ADBond
Copy link
Contributor Author

ADBond commented Oct 22, 2024

Oddly the tests that fail are passing in sqlite - makes sense that the tests are okay in sqlite - no salting, and sampling uses absolute number rather than proportion.

Really it's slightly odd that the postgres tests fail - but looks like that is a separate issue.

ADBond added 14 commits October 23, 2024 13:12
this double-selection (of count_l, count_r - once each in * and once each explicitly) can cause errors in some dialects, in some execution modes
this prevents looking up the wrong table when using debug mode
…broken-clustering

Fix clustering in debug mode
without this, tests become coupled - specifically failures in `tests/test_debug_mode.py::test_debug_mode_clustering` and `tests/test_debug_mode.py::test_debug_mode_cluster_studio` cause failures in `tests/test_u_train.py::test_u_train_link_only` if run beforehand in the same test session
circumvents an issue with parquet method, where empty tables may be cleaned up before they are queried, particularly problematic in debug mode
…session-handling

Spark test session handling
@ADBond ADBond marked this pull request as ready for review November 11, 2024 15:48
@ADBond
Copy link
Contributor Author

ADBond commented Nov 11, 2024

@RobinL - there's nothing particularly major here, but might be worth a glance over.

All of the 'real' changes are in separate PRs with some explanation should you feel the need. Beyond that it might also be useful to look at the test decorator for checking issues with functionality in debug mode, to see if you think that seems a sensible way to go about it.

Copy link
Member

@RobinL RobinL left a comment

Choose a reason for hiding this comment

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

Thanks - had a quick look. Looks good to me. I liked the format of the PR, with the links to component PRs, made it easy to understand!

@ADBond ADBond merged commit 2f67811 into master Nov 12, 2024
25 checks passed
@ADBond ADBond deleted the bug/fixup-debug-mode branch November 12, 2024 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working debug_mode testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants