Skip to content

Conversation

@shapiromatron
Copy link
Collaborator

@shapiromatron shapiromatron commented May 22, 2025

Write new tests, refactor code a little bit, and increase test coverage to 99.5%.

I also updated our dev dependencies to the latest versions.

This is follow-on work from #69

@shapiromatron shapiromatron requested a review from Copilot May 22, 2025 18:35
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR aims to increase test coverage to 99.5% by adding new tests and refactoring parts of the core functionality for both writing and parsing RIS files.

  • Refactored tests across writer, parser, and utils to use fixtures and cover more scenarios.
  • Updated dependency versions and configuration in pyproject.toml.
  • Adjusted core code for writer and parser modules, including the introduction of an abstract method for header generation.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_writer.py Refactored tests to use a fixture for input data and updated expected outputs.
tests/test_utils.py Added tests for the invert_dictionary utility function.
tests/test_parser.py Refactored file-loading tests and added a fixture for expected outcomes.
rispy/writer.py Changed set_header to an abstract method and updated dump/dumps logic.
rispy/parser.py Removed a type-check in _add_list_value and updated load/loads functions.
pyproject.toml Updated dev dependency versions and raised the coverage threshold to 99.5.
Comments suppressed due to low confidence (1)

rispy/parser.py:200

  • Removing the type check for 'record[name]' (previously verifying it was a string) may lead to unintended behavior when 'record[name]' is already a non-string value. Consider reintroducing the type validation to avoid wrapping a non-string value into a new list.
must_exist = record[name]

@shapiromatron shapiromatron requested a review from J535D165 May 22, 2025 18:36
@shapiromatron shapiromatron marked this pull request as ready for review May 22, 2025 18:36
Copy link
Collaborator

@J535D165 J535D165 left a comment

Choose a reason for hiding this comment

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

Very good PR

Base automatically changed from poe-the-poet to main May 23, 2025 15:32
@J535D165
Copy link
Collaborator

Thanks, feel free to merge after the rebase.

# Conflicts:
#	.github/workflows/main.yml
#	README.md
#	pyproject.toml
@shapiromatron shapiromatron merged commit 33c3120 into main May 23, 2025
6 checks passed
@shapiromatron shapiromatron deleted the extra-coverage branch May 23, 2025 16:19
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