Skip to content

Conversation

@mathiusj
Copy link
Contributor

@mathiusj mathiusj commented Dec 9, 2025

Add VCR Infrastructure for DSL Workflow Tests

Added VCR support for DSL workflow tests to enable reliable HTTP request recording and playback.

What changed?

  • Created a new VCRURLRewriter module to handle API endpoint transformations for cassette portability
  • Added a test workflow vcr_test_workflow.rb that makes a simple LLM query about the deepest lake
  • Created a VCR cassette with a recorded response about Lake Baikal
  • Added a new test case that verifies the workflow runs correctly with the VCR cassette
  • Set up environment variables in test setup to ensure tests work in both recording and replay modes

How to test?

In replay mode (default):

bundle exec rake test

To record new cassettes:

OPENAI_API_KEY=your-key RECORD_VCR=true bundle exec rake test

Why make this change?

This change improves test reliability by eliminating dependencies on external API services during testing. By recording API interactions once and playing them back during subsequent test runs, we can:

  • Make tests faster and more deterministic
  • Avoid hitting rate limits or incurring costs from real API calls
  • Test without requiring real API credentials
  • Ensure consistent test behavior regardless of network conditions

Copy link
Contributor Author

mathiusj commented Dec 9, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@mathiusj mathiusj force-pushed the mathiusj/gh-570-add-vcr-support-dsl-tests branch from 8a0efda to 0125231 Compare December 9, 2025 22:22
Copy link
Contributor Author

mathiusj commented Dec 9, 2025

@juniper-shopify updated approach to add VCR support for the functional tests. I can iterate on this a bit more if this looks right to you 🙏

@mathiusj mathiusj force-pushed the mathiusj/gh-570-add-vcr-support-dsl-tests branch 3 times, most recently from 30df328 to 5e039a8 Compare December 10, 2025 19:39
@mathiusj mathiusj marked this pull request as ready for review December 12, 2025 23:05
Copy link
Contributor

Run the new test: OPENAI_API_KEY=api-key-here RECORD_VCR=true bundle exec ruby -Itest test/dsl/functional/roast_dsl_examples_test.rb

This doesn't actually work. You need to also delete the cassette file that you want to re-record.

Copy link
Contributor

@juniper-shopify juniper-shopify left a comment

Choose a reason for hiding this comment

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

You're on the right track here. I think this is good.

A few functional issues to work through, but I like the direction

@mathiusj mathiusj changed the title feat(dsl): add VCR infrastructure for DSL functional tests Add VCR infrastructure for DSL functional tests Dec 17, 2025
@mathiusj mathiusj force-pushed the mathiusj/gh-570-add-vcr-support-dsl-tests branch 9 times, most recently from 3efad62 to 542abbc Compare December 17, 2025 20:59
Copy link
Contributor Author

This doesn't actually work. You need to also delete the cassette file that you want to re-record.

@juniper-shopify I missed this comment earlier as I was focused on the inline comments, so would have to backtrack a bit to see what the issue was that you were getting. Mind sharing the stacktrace or response you were getting so I can see what the issue was? I'm also adding OPENAI_API_BASE=... to run the test with it pointing to our proxy, which does work with the rest of that line.

Please LMK if it works for you now! Also added a new class for rewriting the vcr config, with tests. Given it's EOD EDT I imagine this one may carry over to the new year, so can split this work into a stack for better reviewing/testing, when I get back

@mathiusj mathiusj force-pushed the mathiusj/gh-570-add-vcr-support-dsl-tests branch 4 times, most recently from fae0f42 to 9216de6 Compare January 16, 2026 21:22
@mathiusj mathiusj force-pushed the mathiusj/gh-570-add-vcr-support-dsl-tests branch from 9216de6 to b896a98 Compare January 23, 2026 22:54
Copy link
Contributor

@dersam dersam left a comment

Choose a reason for hiding this comment

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

This PR removes a number of the useful test abstractions from the original functional test system, and reimplements several VCR features. Please refactor.

Copy link
Contributor

dersam commented Jan 26, 2026

This doesn't actually work. You need to also delete the cassette file that you want to re-record.

@juniper-shopify I missed this comment earlier as I was focused on the inline comments, so would have to backtrack a bit to see what the issue was that you were getting. Mind sharing the stacktrace or response you were getting so I can see what the issue was? I'm also adding OPENAI_API_BASE=... to run the test with it pointing to our proxy, which does work with the rest of that line.

Please LMK if it works for you now! Also added a new class for rewriting the vcr config, with tests. Given it's EOD EDT I imagine this one may carry over to the new year, so can split this work into a stack for better reviewing/testing, when I get back

VCR will only record a new cassette if one doesn't already exist. If you need to replace the cassette and hit the proxy live, the old related cassette needs to be deleted. RECORD_VCR allows it to record if missing, without that flag it will simply fail if no cassette exists.

@mathiusj mathiusj force-pushed the mathiusj/gh-570-add-vcr-support-dsl-tests branch 3 times, most recently from 2f4c749 to 1daa26d Compare January 26, 2026 17:16
@mathiusj mathiusj force-pushed the mathiusj/gh-570-add-vcr-support-dsl-tests branch 2 times, most recently from a582347 to e47a46c Compare January 26, 2026 21:46
Copy link
Contributor Author

@dersam if I'm missing anything here now just lmk 🙏

@mathiusj mathiusj requested a review from dersam January 26, 2026 21:54
skip "slow test" unless ["1", "true"].include?(ENV["ROAST_RUN_SLOW_TESTS"])
end

# Set dummy credentials for tests (VCR will filter these in recordings)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the point of overwriting these credentials? When recording, real creds are necessary and will be filtered by filter_sensitive_data. Is there a reason this is needed during replay?

@mathiusj mathiusj force-pushed the mathiusj/gh-570-add-vcr-support-dsl-tests branch 4 times, most recently from 594c8c5 to d4c4ba2 Compare January 29, 2026 22:13
@mathiusj mathiusj marked this pull request as draft January 29, 2026 22:14
@mathiusj mathiusj marked this pull request as ready for review January 29, 2026 22:15
@mathiusj mathiusj force-pushed the mathiusj/gh-570-add-vcr-support-dsl-tests branch from d4c4ba2 to 387f296 Compare January 29, 2026 22:30
@mathiusj mathiusj force-pushed the mathiusj/gh-570-add-vcr-support-dsl-tests branch from 387f296 to bc4a3d5 Compare January 29, 2026 22:45
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