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

feat: add pytest-vcr for recording HTTP interactions in integration tests #2445

Merged
merged 8 commits into from
Apr 7, 2023
Merged

feat: add pytest-vcr for recording HTTP interactions in integration tests #2445

merged 8 commits into from
Apr 7, 2023

Conversation

sergerdn
Copy link
Contributor

@sergerdn sergerdn commented Apr 5, 2023

Using pytest-vcr in integration tests has several benefits. Firstly, it removes the need to mock external services, as VCR records and replays HTTP interactions on the fly. Secondly, it simplifies the integration test setup by eliminating the need to set up and tear down external services in some cases. Finally, it allows for more reliable and deterministic integration tests by ensuring that HTTP interactions are always replayed with the same response.
Overall, pytest-vcr is a valuable tool for simplifying integration test setup and improving their reliability

This commit adds the pytest-vcr package as a dependency for integration tests in the pyproject.toml file. It also introduces two new fixtures in tests/integration_tests/conftest.py files for managing cassette directories and VCR configurations.

In addition, the tests/integration_tests/vectorstores/test_elasticsearch.py file has been updated to use the @pytest.mark.vcr decorator for recording and replaying HTTP interactions.

Finally, this commit removes the documents fixture from the test_elasticsearch.py file and replaces it with a new fixture defined in tests/integration_tests/vectorstores/conftest.py that yields a list of documents to use in any other tests.

This also includes my second attempt to fix issue : #2386

Maybe related #2484

@sergerdn sergerdn changed the title feat: add pytest-vcr feat(tests): add pytest-vcr for recording HTTP interactions with external services Apr 6, 2023
@sergerdn sergerdn changed the title feat(tests): add pytest-vcr for recording HTTP interactions with external services feat: add pytest-vcr for recording HTTP interactions in integration tests Apr 6, 2023
@sergerdn sergerdn marked this pull request as ready for review April 6, 2023 10:24
…ture/add-pytest-vcr

# Conflicts:
#	tests/README.md
@sergerdn sergerdn marked this pull request as draft April 6, 2023 16:01
@sergerdn sergerdn marked this pull request as ready for review April 6, 2023 18:18
@sergerdn sergerdn marked this pull request as draft April 6, 2023 18:51
@sergerdn sergerdn marked this pull request as ready for review April 6, 2023 19:32
@sergerdn
Copy link
Contributor Author

sergerdn commented Apr 6, 2023

@hwchase17

I'm ready for review now.

Copy link
Contributor

@hwchase17 hwchase17 left a comment

Choose a reason for hiding this comment

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

one comment around dependencies - otherwise love it - thanks

pyproject.toml Outdated Show resolved Hide resolved
@sergerdn
Copy link
Contributor Author

sergerdn commented Apr 6, 2023

I am finished. Perhaps I made some mistakes, which I will correct in the next PR if necessary. I have some plans to improve the tests Elastic.

@hwchase17
Copy link
Contributor

I am finished. Perhaps I made some mistakes, which I will correct in the next PR if necessary. I have some plans to improve the tests Elastic.

thanks, that would be great - im excited to try elastic more, so would love any more tests

also, are you on twitter or anything?? i love all your contributions, want to give you a shout out :)

@sergerdn
Copy link
Contributor Author

sergerdn commented Apr 7, 2023

I am finished. Perhaps I made some mistakes, which I will correct in the next PR if necessary. I have some plans to improve the tests Elastic.

thanks, that would be great - im excited to try elastic more, so would love any more tests

also, are you on twitter or anything?? i love all your contributions, want to give you a shout out :)

Thanks for your message! I just want to clarify that I'm not a professional programmer, but I do have some technical skills. I'm glad to hear that you're excited to try out Elastic more and conduct more tests. I should mention that I'm not a big specialist with Elastic myself, but I'm happy to help in any way I can. Unfortunately, I'm not on Twitter or any other social media platforms.

However, I have a friend who is more experienced with Elastic and may be able to provide more specialised support and guidance.

P.S.
If you would like to contact me personally, please leave a comment below with your email address. I will receive an email notification from GitHub. If you would like to keep your email private, you can delete it a few minutes after sending your message.

@sergerdn
Copy link
Contributor Author

sergerdn commented Apr 7, 2023

If possible, please merge those changes as soon as possible because I need to continue working. Thank you very much for your understanding.

@hwchase17 hwchase17 merged commit 6dc86ad into langchain-ai:master Apr 7, 2023
@sergerdn sergerdn deleted the feature/add-pytest-vcr branch April 7, 2023 15:22
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.

None yet

2 participants