langchainrb
langchainrb copied to clipboard
Rspec 'integration' tests for optional use by contributors and ci
This isn't done, but I'm looking for feedback on using either cucumber or rspec for integration tests, which would aid in BDD development and be run before releases. If rspec is chosen, we can configure it to skip the 'integration' folder unless requested.
This PR includes both options for comparison.
For #144
One difficulty here is I am getting several different answers for the tests, so what's the acceptance criteria? Just absence of errors? 5,845 soccer fields 204 soccer fields 4,125 soccer fields
@mattlindsey Isn't RSpec enough for testing library code? Why do we need cucumber
it doesn't seem to bring additional value to what we can already do with rspec testing an Agent.
@alchaplinsky Yeah. I'll redo this proposal to only include the rspec file, exclude the 'integrations' folder when running rspec, and add a short note in the README suggesting contributors utilize the specs directly.
@alchaplinsky. Is this better? I think they're useful.
@mattlindsey I think integration tests for concept like Agent is a good idea. But we need to mock requests to external APIs. I don't think that anyone would want to pay for tokens used during test runs.
Personally I wanted these to aid in development, but I can just keep them for myself. Existing tests already do the mocking.
@alchaplinsky Sorry, I meant to tag you on the last comment. I added a line in the README to clarify the intent of the directory and files.
Ok, got it. So this is a type of test that actually runs against real APIs and makes sure that code works with real integrations. Thus we skip running them every time when we run a test suite and we can enable them by setting the INTEGRATION
env variable. Is that right? I'd probably name that variable something more descriptive like:
INTEGRATION_TESTING = true
INTEGRATION_TESTS_ENABLED = true
Otherwise, it is not entirely clear what INTEGRATION
stands for.
INTEGRATION_TESTS_ENABLED = true
@alchaplinsky Yeah. That's better. Done
Closing this to avoid too many open PRs but leaving issue #144 open.
I'm reopening this PR because I think it's a good idea to have these optional 'integration' tests, especially for use when developing langchainrb itself. It's quicker and less error prone than using bin/console
.
@andreibondarev I can update this PR to test Langchain::Assistant
instead of ChainOfThought if you agree this might be useful.