chore(langchain): fix flaky cassette tests, skipping logic #9768
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Yun-Kim
added
the
changelog/no-changelog
Yun-Kim
force-pushed
the
yunkim/langchain-fix-flaky
branch
from
ddcf4a1
to
c139f43
Compare
Datadog ReportBranch report: ✅ 0 Failed, 137477 Passed, 40702 Skipped, 7h 46m 59.24s Total duration (3m 2.1s time saved) |
BenchmarksBenchmark execution time: 2024-07-29 21:54:17 Comparing candidate commit 3b416e1 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 214 metrics, 2 unstable metrics. |
Yun-Kim
force-pushed
the
yunkim/langchain-fix-flaky
branch
from
33f1650
to
f6d7302
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9768 +/- ##
===========================================
- Coverage 74.48% 10.42% -64.07%
===========================================
Files 1391 1358 -33
Lines 128824 126595 -2229
===========================================
- Hits 95959 13193 -82766
- Misses 32865 113402 +80537 ☔ View full report in Codecov by Sentry. |
|
|
16 tasks
Yun-Kim
force-pushed
the
yunkim/langchain-fix-flaky
branch
from
cc78aad
to
489597a
Compare
Yun-Kim
force-pushed
the
yunkim/langchain-fix-flaky
branch
from
489597a
to
1b35dc4
Compare
Yun-Kim
force-pushed
the
yunkim/langchain-fix-flaky
branch
from
5e0023c
to
d35e50e
Compare
2 tasks
Yun-Kim
changed the title
…or langchain v0 tests
Yun-Kim
force-pushed
the
yunkim/langchain-fix-flaky
branch
from
1ddb905
to
128dd49
Compare
Yun-Kim
force-pushed
the
yunkim/langchain-fix-flaky
branch
from
128dd49
to
2c5b38c
Compare
Yun-Kim
force-pushed
the
yunkim/langchain-fix-flaky
branch
from
e7c7e0a
to
6ef4e9a
Compare
sabrenner
approved these changes
Jul 30, 2024
lievan
approved these changes
Jul 30, 2024
2 tasks
Yun-Kim
added a commit
that referenced
this pull request
Jul 31, 2024
…e for CI (#9997) Even after #9768 we're still seeing some tests be skipped on CI, specifically `langchain-community` tests even when `langchain>=0.1`. My suspicion is that the `sys.version_info < (0, 1, 0)` conditional that is used for the test skipping is too precise for the versions of Langchain being run on CI (not exactly sure what the issue is, but I've narrowed it down to that area). This PR only checks the major and minor version of Langchain being run ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR attempts to address the flakiness and general (lack of) reliability of the LangChain test suite, and removes the flaky test markers from most langchain tests. No functionality has been changed, this PR focuses only on improving our test suite to be less flaky and be a better signal of our langchain library coverage.
Problems with LangChain test suite:
This PR solves the above problems by:
vcrpyto version5.1.0which is the version prior to the linked issue being introducedPATCH_LANGCHAIN_V0constant from the patch file which appeared to not be truthful)(3, 10, 0)as this may result in some edge cases. Do not skip Python 3.9 testing ontest_langchain_community.pyor community tests ontest_langchain_llmobs.py.We have made some fuzzy approaches to handling Python 3.9 and langchain v0:
langchain v0tests and onelangchain v1test due to the cassette file being different across python 3.9 and python 3.10+. To avoid generating so many test cassette files, we have skipped these handful of tests only for Python 3.9.langchain (v0)andlangchain_community (v1+), and only run those tests for respective langchain version environments.Checklist
Reviewer Checklist