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

CLI Fixes #322

Merged
merged 7 commits into from
Sep 9, 2024
Merged

CLI Fixes #322

merged 7 commits into from
Sep 9, 2024

Conversation

whitead
Copy link
Collaborator

@whitead whitead commented Sep 8, 2024

  • modify/saving settings
  • asking queries
  • building indexes
  • search answer index
  • searching other index

@whitead whitead marked this pull request as draft September 8, 2024 06:38
@whitead whitead changed the title Added CLI tests CLI Improvements Sep 8, 2024
@whitead
Copy link
Collaborator Author

whitead commented Sep 8, 2024

Finished writing the tests - they fail 👍

@whitead whitead changed the title CLI Improvements CLI Fixes Sep 9, 2024
@whitead whitead marked this pull request as ready for review September 9, 2024 13:32
@whitead
Copy link
Collaborator Author

whitead commented Sep 9, 2024

Hey @mskarlin - I cannot fix the last unit test. It seems to rely on some JSON file being present, but none are being saved to the index. I cannot understand the logic of them being saved in some cases and not in others. I'm hoping you have some knowledge of this.

pytest tests/test_cli.py -k build  

will run the test

…tter robust to having a Doc object, condition index type on being an "answers" index
@@ -115,7 +116,7 @@ async def query(self, query: dict) -> DocDetails | None:
f"{client_query.doi if isinstance(client_query, DOIQuery) else client_query.title}"
f" in {self.__class__.__name__}."
)
except TimeoutError:
except (TimeoutError, asyncio.exceptions.TimeoutError):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a comment here saying after Python>=3.11 TimeoutError replaces asyncio.exceptions.TimeoutError

@mskarlin mskarlin merged commit a9738c8 into september-2024-release Sep 9, 2024
1 of 2 checks passed
@mskarlin mskarlin deleted the cli-tests branch September 9, 2024 18:28
@whitead whitead mentioned this pull request Sep 11, 2024
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