Skip to content

Make dealership_adv8 Test For SQL Files Check Deterministic #508

Merged
hadia206 merged 4 commits intomainfrom
Hadia/fix_dealership_adv8_sql_file_test
Apr 9, 2026
Merged

Make dealership_adv8 Test For SQL Files Check Deterministic #508
hadia206 merged 4 commits intomainfrom
Hadia/fix_dealership_adv8_sql_file_test

Conversation

@hadia206
Copy link
Copy Markdown
Contributor

@hadia206 hadia206 commented Apr 8, 2026

  • Fix flaky dealership_adv8 SQL files test: impl_defog_dealership_adv8 asks “what are the sales metrics for the last 6 months?” It computes those 6 month boundaries in Python using today’s date, then adds them as hardcoded timestamps into the generated SQL (e.g. VALUES ('2025-10-01'), ('2025-11-01'), ...). The SQL comparison test works by saving that generated SQL to a reference file and comparing future runs against it. Since the 6-month window shifts every month, the reference file goes stale on the 1st of every month and the SQL file test fails.

  • Solution: Add a fixed_today: pd.Timestamp | None field to PyDoughSQLComparisonTest. When set, pd.Timestamp.today is patched during SQL generation so the output is deterministic. The reference files are pinned to the Oct 2025–Mar 2026 window (today = 2026-04-01). Execution tests are unaffected, they use SQL-native date expressions (DATE('now', ...)).

AND

Migrate [tool.uv] dev-dependencies to [dependency-groups] dev in pyproject.toml to silence a uv deprecation warning.

uses: astral-sh/setup-uv@v3
with:
version: "0.4.23"
version: "0.6.0"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

needed to update version to use new pyproject Syntax and get rid of the warning message.

@hadia206 hadia206 marked this pull request as ready for review April 9, 2026 17:49
@hadia206 hadia206 requested review from a team, john-sanchez31, juankx-bodo and knassre-bodo and removed request for a team April 9, 2026 17:49
# Pin today so the generated VALUES clause (6 month-start
# timestamps) stays deterministic. Reference files reflect
# the Oct 2025 – Mar 2026 window (today = 2026-04-01).
fixed_today=pd.Timestamp("2026-04-01"),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This this would make the same 6 month range each time, wouldn't this be a problem next month for the expected result? Because defog_sql_text_dealership_adv8 changes every month (build dynamically) same with the defog data that uses current_date

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's the point of the PR. We are fixing the date to be the same for generating the exact same SQL files. It's not impacting the execution and the expected results of the query itself.

The key is that both sides of the comparison are now frozen to the same fixed point for SQL file tests only.
fixed_today patches pd.Timestamp.today() to always return 2026-04-01, so the generated SQL will always produce the Oct 2025–Mar 2026 window. The reference .sql files are hardcoded that same window. Since both sides are pinned to the same value, the test will pass every month.

The execution test (test_defog_e2e) is separate and unaffected. It uses SQL-native DATE('now', ...) expressions that compute relative to the actual current date, so it always queries the real current 6-month window from the live database.

@hadia206 hadia206 requested a review from john-sanchez31 April 9, 2026 19:14
Copy link
Copy Markdown
Contributor

@john-sanchez31 john-sanchez31 left a comment

Choose a reason for hiding this comment

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

If it doesn't affect the e2e tests we're good

@juankx-bodo
Copy link
Copy Markdown
Contributor

LGTM.

@hadia206 hadia206 merged commit ef2a89d into main Apr 9, 2026
15 checks passed
@hadia206 hadia206 deleted the Hadia/fix_dealership_adv8_sql_file_test branch April 9, 2026 21:48
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.

3 participants