Skip to content

[APP-345] feat: sas, scriptifying test harness for use outside of tests#3105

Merged
mcmcgrath13 merged 10 commits intomainfrom
mcm/report-env-compare
Apr 14, 2026
Merged

[APP-345] feat: sas, scriptifying test harness for use outside of tests#3105
mcmcgrath13 merged 10 commits intomainfrom
mcm/report-env-compare

Conversation

@mcmcgrath13
Copy link
Copy Markdown
Contributor

@mcmcgrath13 mcmcgrath13 commented Mar 25, 2026

Description

  • Make test harness for loading fake data also available via script
  • Add SAS container to build options
  • Simplify API for using the table faker harness

Notes:

Tickets

Checklist before requesting a review

  • PR focuses on a single story
  • Code has been fully tested to meet acceptance criteria
  • PR is reasonably small and reviewable (Generally less than 10 files and 500 changed lines)
  • All new functions/classes/components reasonably small
  • Functions/classes/components focused on one responsibility
  • Code easy to understand and modify (clarity over concise/clever)
  • PRs containing TypeScript follow the Do's and Don'ts
  • PR does not contain hardcoded values (Uses constants)
  • All code is covered by unit or feature tests

@mcmcgrath13 mcmcgrath13 changed the title feat: wip - scriptifying test harness for use outside of tests feat: sas, scriptifying test harness for use outside of tests Apr 13, 2026
@mcmcgrath13 mcmcgrath13 marked this pull request as ready for review April 14, 2026 00:00
@mcmcgrath13 mcmcgrath13 requested a review from a team as a code owner April 14, 2026 00:00
@mcmcgrath13 mcmcgrath13 requested review from JordanGuinn and emyl3 and removed request for a team April 14, 2026 00:00
@mcmcgrath13 mcmcgrath13 changed the title feat: sas, scriptifying test harness for use outside of tests [APP-345] feat: sas, scriptifying test harness for use outside of tests Apr 14, 2026
Copy link
Copy Markdown
Contributor

@JordanGuinn JordanGuinn left a comment

Choose a reason for hiding this comment

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

One q about a potential env gate for the data scripts, but otherwise looks good to me! 🚀

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.

(thought, b): Are there any build parameters or env vars we can take advantage of here to ensure this and the restore_original.data.py can only ever be run in dev/pre-prod environments?

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.

These files (scripts or tests) aren't included in our built container, which I think is our best proxy for prod. Given a human would need to set up an env var for the connection to a prod database, then also pass an argument, it feels like it would need to be pretty intentional. And the good news is they could maybe quickly restore if they did this by accident?

I don't think there's any way to know from a DB itself if it's production or not 🤔

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.

Alrighty. Something to keep an eye on moving forward I'd think, should we ever expand usage of that env var, or need to incorporate any Python scripts into the build for whatever reason

yield

# restore the original data
restore_original_data(conn_string, db_tables, fk_tables)
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.

TIL that the therestore_original_data call after the yield indicates to pytest that it's teardown code! For a sec I was confused as to why we were inserting fake data just to immediately truncate it 😅 🤦

Copy link
Copy Markdown
Contributor

@emyl3 emyl3 left a comment

Choose a reason for hiding this comment

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

worked as expected for me!

@mcmcgrath13 mcmcgrath13 merged commit 8884cf1 into main Apr 14, 2026
3 checks passed
@mcmcgrath13 mcmcgrath13 deleted the mcm/report-env-compare branch April 14, 2026 21:33
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