Skip to content

🧪 Add tests for save_results in utils.py#31

Open
leo-gan wants to merge 2 commits intomainfrom
testing-improvement-save-results-12574618920439182633
Open

🧪 Add tests for save_results in utils.py#31
leo-gan wants to merge 2 commits intomainfrom
testing-improvement-save-results-12574618920439182633

Conversation

@leo-gan
Copy link
Copy Markdown
Owner

@leo-gan leo-gan commented Apr 22, 2026

Added tests/test_utils.py to test the save_results function in packages/pdf-anonymizer-core/src/pdf_anonymizer_core/utils.py. The tests verify that the function correctly creates directories, handles file extensions (especially the .pdf to .md conversion), and saves the expected content to the correct locations. Used monkeypatch to redirect file operations to a temporary directory during testing.


PR created automatically by Jules for task 12574618920439182633 started by @leo-gan

This commit adds unit tests for the `save_results` function in `pdf_anonymizer_core.utils`.
The new tests cover:
- Directory creation for anonymized text and mappings.
- File naming and extension conversion (e.g., .pdf -> .md).
- Persistence of non-PDF file extensions.
- Correctness of saved content for both text and JSON mapping.

Tests use `pytest` with `tmp_path` and `monkeypatch.chdir` to ensure isolation and prevent side effects in the repository.
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces unit tests for the save_results utility, covering both PDF and TXT file formats. Feedback includes removing an unused Path import and enhancing the TXT test case to include missing assertions for directory creation and file content, potentially through test parametrization.

Comment thread tests/test_utils.py Outdated
@@ -0,0 +1,55 @@
import json
import os
from pathlib import Path
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The Path import is unused in this file and can be removed to maintain code cleanliness.

Comment thread tests/test_utils.py
Comment on lines +48 to +55
anonymized_output_file, mapping_file = save_results(
full_anonymized_text, final_mapping, file_path
)

# Check if file extension is preserved
expected_anonymized_file = "data/anonymized/test_document.anonymized.txt"
assert anonymized_output_file == expected_anonymized_file
assert os.path.exists(expected_anonymized_file)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The test_save_results_txt function is incomplete compared to test_save_results_pdf. It fails to verify the directory creation, the mapping_file path and existence, and the content of the generated files. Additionally, the mapping_file variable is currently unused.

Consider using pytest.mark.parametrize to combine these tests into a single, more maintainable test function that covers both file extensions with full assertions.

    anonymized_output_file, mapping_file = save_results(
        full_anonymized_text, final_mapping, file_path
    )

    # Check if directories were created
    assert os.path.exists("data/anonymized")
    assert os.path.exists("data/mappings")

    # Check if file extension is preserved and mapping is saved
    expected_anonymized_file = "data/anonymized/test_document.anonymized.txt"
    expected_mapping_file = "data/mappings/test_document.mapping.json"

    assert anonymized_output_file == expected_anonymized_file
    assert mapping_file == expected_mapping_file
    assert os.path.exists(expected_anonymized_file)
    assert os.path.exists(expected_mapping_file)

    # Check content of the anonymized file
    with open(expected_anonymized_file, "r", encoding="utf-8") as f:
        assert f.read() == full_anonymized_text

    # Check content of the mapping file
    with open(expected_mapping_file, "r", encoding="utf-8") as f:
        assert json.load(f) == final_mapping

This commit adds unit tests for the `save_results` function in `pdf_anonymizer_core.utils`.
The new tests cover:
- Directory creation for anonymized text and mappings.
- File naming and extension conversion (e.g., .pdf -> .md).
- Persistence of non-PDF file extensions.
- Correctness of saved content for both text and JSON mapping.

Tests use `pytest` with `tmp_path` and `monkeypatch.chdir` to ensure isolation and prevent side effects in the repository. Unused `Path` import was removed from the test file to satisfy linting requirements.
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.

1 participant