-
Notifications
You must be signed in to change notification settings - Fork 551
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
Cleaning up and testing get_directory_index
#483
Conversation
5ed0a39
to
a5399aa
Compare
@@ -343,7 +343,7 @@ async def maybe_get_manifest( | |||
async with await anyio.open_file(filename, mode="r") as file: | |||
content = await file.read() | |||
records = [DocDetails(**row) for row in csv.DictReader(StringIO(content))] | |||
logger.info( | |||
logger.debug( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a bit noisy, so downgraded its level
paper_dir = cast(Path, agent_test_settings.paper_directory) | ||
assert results[0].docs.keys() == {md5sum((paper_dir / "bates.txt").absolute())} | ||
# Use a tempdir here so we can delete files without affecting concurrent tests | ||
with tempfile.TemporaryDirectory() as tempdir: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have thought this fixture would handle that -- did you see deleting files affecting other tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah good question. I just expanded the explanatory comment to address this:
# Since agent_test_settings is used by other tests, and thus uses the same
# paper_directory as other tests, we use a tempdir so we can delete files
# without affecting concurrent tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
I found this code somewhat confusing, so I: