From 47c6799be89ab499fb8368f9b79a8e8785622d33 Mon Sep 17 00:00:00 2001 From: Ritesh Rana <44251619+ambicuity@users.noreply.github.com> Date: Thu, 23 Oct 2025 01:44:00 -0500 Subject: [PATCH 1/4] test: Add unit tests for encrypt_pdf function Add unit tests for encrypt_pdf function to verify functionality, including output file creation, encryption status, decryption with correct password, page count preservation, and handling of invalid input. --- tests/unittests/test_encryptpdf.py | 89 ++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/tests/unittests/test_encryptpdf.py b/tests/unittests/test_encryptpdf.py index 83fc7df..06be40c 100644 --- a/tests/unittests/test_encryptpdf.py +++ b/tests/unittests/test_encryptpdf.py @@ -6,3 +6,92 @@ class TestPdfUnitTests(TestCase): def test_pipeline(self): self.assertEquals("awesomePdfProtection-UnitTest", pipeline("awesomePdfProtection-UnitTest")) + + +from encryptpdf import encrypt_pdf +import os +import tempfile +from pypdf import PdfReader + + +class TestEncryptPdf(TestCase): + """Unit tests for the encrypt_pdf function.""" + + def setUp(self): + """Set up test fixtures.""" + self.test_password = "test_password_123" + self.temp_dir = tempfile.mkdtemp() + self.input_pdf = os.path.join( + os.path.dirname(__file__), + "..", + "resources", + "Sample_PDF.pdf" + ) + self.output_pdf = os.path.join(self.temp_dir, "encrypted_output.pdf") + + def tearDown(self): + """Clean up test files.""" + if os.path.exists(self.output_pdf): + os.remove(self.output_pdf) + if os.path.exists(self.temp_dir): + os.rmdir(self.temp_dir) + + def test_encrypt_pdf_creates_output_file(self): + """Test that encrypt_pdf creates an output file.""" + encrypt_pdf(self.input_pdf, self.output_pdf, self.test_password) + self.assertTrue( + os.path.exists(self.output_pdf), + "Encrypted PDF file should be created" + ) + + def test_encrypt_pdf_output_is_encrypted(self): + """Test that the output PDF is actually encrypted.""" + encrypt_pdf(self.input_pdf, self.output_pdf, self.test_password) + + # Try to read without password - should indicate encryption + reader = PdfReader(self.output_pdf) + self.assertTrue( + reader.is_encrypted, + "Output PDF should be encrypted" + ) + + def test_encrypt_pdf_with_correct_password(self): + """Test that encrypted PDF can be decrypted with correct password.""" + encrypt_pdf(self.input_pdf, self.output_pdf, self.test_password) + + reader = PdfReader(self.output_pdf) + # Decrypt with correct password + decrypt_result = reader.decrypt(self.test_password) + + self.assertNotEqual( + decrypt_result, + 0, + "Should be able to decrypt with correct password" + ) + + def test_encrypt_pdf_preserves_page_count(self): + """Test that encryption preserves the number of pages.""" + # Read original page count + original_reader = PdfReader(self.input_pdf) + original_page_count = len(original_reader.pages) + + # Encrypt + encrypt_pdf(self.input_pdf, self.output_pdf, self.test_password) + + # Read encrypted page count + encrypted_reader = PdfReader(self.output_pdf) + encrypted_reader.decrypt(self.test_password) + encrypted_page_count = len(encrypted_reader.pages) + + self.assertEqual( + original_page_count, + encrypted_page_count, + "Page count should be preserved after encryption" + ) + + def test_encrypt_pdf_with_invalid_input(self): + """Test encrypt_pdf with non-existent input file.""" + non_existent_file = "non_existent_file.pdf" + + with self.assertRaises(FileNotFoundError): + encrypt_pdf(non_existent_file, self.output_pdf, self.test_password) From a8e16f302afe643002239aa83f73eb0308eff2ae Mon Sep 17 00:00:00 2001 From: riteshr19 Date: Fri, 20 Feb 2026 09:37:22 -0600 Subject: [PATCH 2/4] feat: Add dynamic version flag, handle output collisions, add CONTRIBUTING.md --- CONTRIBUTING.md | 61 ++++++++++++++++++++++++++++ passifypdf/cli.py | 9 ++++- passifypdf/encryptpdf.py | 7 ++++ tests/unittests/test_encryptpdf.py | 64 ++++++++++++++++++++++++++++++ 4 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..017db4b --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,61 @@ +# Contributing to passifypdf + +First off, thank you for considering contributing to passifypdf! It's people like you that make open source such a great community. + +## Development Environment Setup + +This project uses `uv` for dependency management and running tools. + +### 1. Install `uv` +If you haven't already, install [uv](https://github.com/astral-sh/uv). +```bash +curl -LsSf https://astral.sh/uv/install.sh | sh +``` + +### 2. Fork and Clone +Fork the repository and clone it to your local machine: +```bash +git clone https://github.com/YOUR_USERNAME/passifypdf.git +cd passifypdf +``` + +### 3. Install Dependencies +You can install the dependencies using `uv`. +```bash +uv sync +``` +This will set up the virtual environment with all required run and development dependencies. + +## Testing + +We use `pytest` for running our test suite. + +To run tests: +```bash +uv run pytest +``` + +To run tests with coverage reporting: +```bash +uv run pytest --cov=passifypdf +``` +Please make sure all existing tests pass and write new tests for your newly added code before submitting a Pull Request. + +## Code Constraints and Formatting + +Currently, the project strives for PEP 8 compliance. While we may add automated pre-commit formatting hooks in the future, please try to: +- Follow standard Python styling conventions (use `flake8` or `ruff` to identify issues). +- Include standard docstrings for all new functions/classes. +- Ensure your typed logic has appropriate type-hints. + +## Submitting a Pull Request + +1. Create a feature branch from your fork: `git checkout -b feature/your-feature-name`. +2. Commit your changes: `git commit -m "feat: Add your feature text"`. +3. Push to your branch: `git push origin feature/your-feature-name`. +4. Submit a Pull Request on the main repository via GitHub. + +Please link any relevant open issues to your Pull Request explicitly (e.g. "Closes #123"). +CodeRabbit / Copilot AI might review your PR; please address any review feedback they provide. + +Thank you! diff --git a/passifypdf/cli.py b/passifypdf/cli.py index 7364188..de0245d 100644 --- a/passifypdf/cli.py +++ b/passifypdf/cli.py @@ -1,13 +1,20 @@ import argparse +from importlib.metadata import version, PackageNotFoundError def get_arg_parser() -> argparse.ArgumentParser: + try: + __version__ = version("passifypdf") + except PackageNotFoundError: + __version__ = "unknown" + arg_parser = argparse.ArgumentParser( description="Encrypt a PDF file with a password of your choice.", epilog="For more information, visit: https://github.com/SUPAIDEAS/passifypdf" ) - arg_parser.add_argument("-v", "--version", action="version", version="%(prog)s 1.0") + arg_parser.add_argument("-v", "--version", action="version", version=f"%(prog)s {__version__}") arg_parser.add_argument("-i", "--input", required=True, help="Path to the input PDF file to be encrypted") arg_parser.add_argument("-o", "--output", required=True, help="Path where the encrypted PDF file will be saved") arg_parser.add_argument("-p", "--passwd", required=True, type=str, help="Password to encrypt the PDF file with") + arg_parser.add_argument("-f", "--force", action="store_true", help="Overwrite the output file if it already exists without prompting") return arg_parser \ No newline at end of file diff --git a/passifypdf/encryptpdf.py b/passifypdf/encryptpdf.py index e326cab..13de80c 100644 --- a/passifypdf/encryptpdf.py +++ b/passifypdf/encryptpdf.py @@ -60,6 +60,13 @@ def main() -> int: args = arg_parser.parse_args() try: + output_path = Path(args.output) + if output_path.exists() and not getattr(args, 'force', False): + response = input(f"File '{args.output}' already exists. Overwrite? [y/N]: ") + if response.lower() not in ('y', 'yes'): + print("Operation cancelled.") + return 0 + encrypt_pdf(args.input, args.output, args.passwd) print(f"Congratulations!\nPDF file encrypted successfully and saved as '{args.output}'") return 0 diff --git a/tests/unittests/test_encryptpdf.py b/tests/unittests/test_encryptpdf.py index 3ebc03c..61b2556 100644 --- a/tests/unittests/test_encryptpdf.py +++ b/tests/unittests/test_encryptpdf.py @@ -141,3 +141,67 @@ def test_encrypt_pdf_is_directory(self, mock_path_cls): with self.assertRaises(IsADirectoryError): encrypt_pdf("directory", "output.pdf", "secret") + + @patch('passifypdf.encryptpdf.get_arg_parser') + @patch('passifypdf.encryptpdf.encrypt_pdf') + @patch('passifypdf.encryptpdf.Path') + @patch('builtins.print') + def test_main_with_force_flag(self, mock_print, mock_path_cls, mock_encrypt_pdf, mock_arg_parser): + """Test main() with --force flag when output file exists.""" + mock_parser_instance = mock_arg_parser.return_value + mock_args = type('Args', (), {'input': 'in.pdf', 'output': 'out.pdf', 'passwd': 'pass', 'force': True})() + mock_parser_instance.parse_args.return_value = mock_args + + mock_path_instance = mock_path_cls.return_value + mock_path_instance.exists.return_value = True + + from passifypdf.encryptpdf import main + result = main() + + self.assertEqual(result, 0) + mock_encrypt_pdf.assert_called_with('in.pdf', 'out.pdf', 'pass') + + @patch('passifypdf.encryptpdf.get_arg_parser') + @patch('passifypdf.encryptpdf.encrypt_pdf') + @patch('passifypdf.encryptpdf.Path') + @patch('builtins.print') + @patch('builtins.input') + def test_main_without_force_flag_user_says_yes(self, mock_input, mock_print, mock_path_cls, mock_encrypt_pdf, mock_arg_parser): + """Test main() without --force flag, user agrees to overwrite.""" + mock_parser_instance = mock_arg_parser.return_value + mock_args = type('Args', (), {'input': 'in.pdf', 'output': 'out.pdf', 'passwd': 'pass', 'force': False})() + mock_parser_instance.parse_args.return_value = mock_args + + mock_path_instance = mock_path_cls.return_value + mock_path_instance.exists.return_value = True + + mock_input.return_value = 'y' + + from passifypdf.encryptpdf import main + result = main() + + self.assertEqual(result, 0) + mock_encrypt_pdf.assert_called_with('in.pdf', 'out.pdf', 'pass') + + @patch('passifypdf.encryptpdf.get_arg_parser') + @patch('passifypdf.encryptpdf.encrypt_pdf') + @patch('passifypdf.encryptpdf.Path') + @patch('builtins.print') + @patch('builtins.input') + def test_main_without_force_flag_user_says_no(self, mock_input, mock_print, mock_path_cls, mock_encrypt_pdf, mock_arg_parser): + """Test main() without --force flag, user refuses to overwrite.""" + mock_parser_instance = mock_arg_parser.return_value + mock_args = type('Args', (), {'input': 'in.pdf', 'output': 'out.pdf', 'passwd': 'pass', 'force': False})() + mock_parser_instance.parse_args.return_value = mock_args + + mock_path_instance = mock_path_cls.return_value + mock_path_instance.exists.return_value = True + + mock_input.return_value = 'n' + + from passifypdf.encryptpdf import main + result = main() + + self.assertEqual(result, 0) + mock_encrypt_pdf.assert_not_called() + mock_print.assert_called_with("Operation cancelled.") From 6d9e1307d5039a79e1a0eaf8187373e0a053bdbe Mon Sep 17 00:00:00 2001 From: riteshr19 Date: Fri, 20 Feb 2026 09:57:15 -0600 Subject: [PATCH 3/4] test: Address inline code review feedback from Copilot --- create_issues.sh | 42 ++++++++++++++++++++++++++++++ passifypdf/encryptpdf.py | 2 +- tests/unittests/test_cli.py | 42 ++++++++++++++++++++++++++++++ tests/unittests/test_encryptpdf.py | 28 +++++++++++++++++++- 4 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 create_issues.sh create mode 100644 tests/unittests/test_cli.py diff --git a/create_issues.sh b/create_issues.sh new file mode 100644 index 0000000..5ba5dac --- /dev/null +++ b/create_issues.sh @@ -0,0 +1,42 @@ +#!/bin/bash + +echo "Creating Buf issues..." +# Bugs +gh issue create --repo SUPAIDEAS/passifypdf --title "Bug: Special characters and emojis in PDF file paths cause minor complaints" --body "As mentioned in the README, if the user has special characters like an emoji in the PDF file name, the shell gives a minor complain during execution. This should be gracefully handled by encoding the paths correctly or catching the exception and returning a friendly error." --label bug +gh issue create --repo SUPAIDEAS/passifypdf --title "Bug: Validate if the input file is actually a valid PDF format" --body "Currently, the script might attempt to encrypt non-PDF files if passed with a .pdf extension or no extension. We should use a magic byte checker or a try-catch block during the PdfReader initialization to provide a clear error message that the file is not a valid PDF." --label bug +gh issue create --repo SUPAIDEAS/passifypdf --title "Bug: High memory consumption on very large PDF files" --body "The current approach iterates through all pages and adds them to the writer using reader.pages, holding everything in memory. For a multi-gigabyte PDF, this might lead to Out Of Memory (OOM) errors. We should explore optimizing memory usage." --label bug +gh issue create --repo SUPAIDEAS/passifypdf --title "Bug: Output path collision should prompt before overwriting" --body "If the user specifies an output path that already exists, the script silently overwrites it. It should prompt the user (e.g., 'File exists, overwrite? [y/N]') unless a --force flag is provided." --label bug +gh issue create --repo SUPAIDEAS/passifypdf --title "Bug: Handle encrypted PDFs gracefully if passed as input" --body "If the input PDF is already encrypted, pypdf will throw an error when trying to read the pages. The script should catch this specific error and inform the user." --label bug + +echo "Creating Enhancement issues..." +# Enhancements +gh issue create --repo SUPAIDEAS/passifypdf --title "Enhancement: Add an option to decrypt an already protected PDF" --body "It would be useful if passifypdf could also act as a decryption tool if the user provides the correct password and a --decrypt flag." --label enhancement +gh issue create --repo SUPAIDEAS/passifypdf --title "Enhancement: Support setting 256-bit AES encryption explicitly" --body "By default, pypdf might use 128-bit encryption based on the version. We should allow advanced users to specify the encryption algorithm explicitly, e.g., --encryption-level 256." --label enhancement +gh issue create --repo SUPAIDEAS/passifypdf --title "Enhancement: Add a --quiet or -q flag for silent execution" --body "For scripting and CI/CD pipelines, users might not want the 'Congratulations!' message printed to the console. A --quiet flag should suppress all non-error output." --label enhancement +gh issue create --repo SUPAIDEAS/passifypdf --title "Enhancement: Add batch processing support for directories" --body "Allow the input -i to be a directory instead of a file, parsing all .pdf files and creating protected versions in the output directory." --label enhancement +gh issue create --repo SUPAIDEAS/passifypdf --title "Enhancement: Display progress bar during encryption" --body "For PDFs with thousands of pages, the script might take a while. Adding a simple progress bar using tqdm or rich would improve the UX considerably." --label enhancement + +echo "Creating Documentation issues..." +# Documentation +gh issue create --repo SUPAIDEAS/passifypdf --title "Docs: Add a comprehensive CONTRIBUTING.md guide" --body "We need a standard setup guide for contributors, detailing how to set up the local environment, run tests with pytest, and linting rules." --label documentation +gh issue create --repo SUPAIDEAS/passifypdf --title "Docs: Update README with GitHub Actions Status Badges" --body "Adding visually appealing badges for CI/CD status, PyPI version, and License will make the project look more professional." --label documentation +gh issue create --repo SUPAIDEAS/passifypdf --title "Docs: Write a SECURITY.md explaining PDF encryption limits" --body "As noted in the README, PDF encryption should not be relied upon for mission-critical data. A SECURITY.md file detailing the exact cryptographic standards used would help users." --label documentation +gh issue create --repo SUPAIDEAS/passifypdf --title "Docs: Provide examples of using passifypdf programmatically" --body "While the CLI is documented, other Python developers might want to import encrypt_pdf directly in their code. Add a small Python API section to the README." --label documentation +gh issue create --repo SUPAIDEAS/passifypdf --title "Docs: Document expected exit codes and error scenarios" --body "CLI tools should ideally document their exit codes (e.g., 0 for success, 1 for generic error, 2 for missing file). Please add a section in docs/CLI_OPTIONS.md for this." --label documentation + +echo "Creating Good First issues..." +# Good First Issue +gh issue create --repo SUPAIDEAS/passifypdf --title "Refactoring: Migrate argparse to click or typer" --body "The current CLI uses standard library argparse. Migrating to typer or click would provide better automatic help generation, colors, and type enforcement." --label "good first issue" +gh issue create --repo SUPAIDEAS/passifypdf --title "Feature: Add --version / -v flag to CLI" --body "Currently, there is no quick way to check the installed tool version. A standard --version flag should be added reading the version from pyproject.toml or __init__.py." --label "good first issue" +gh issue create --repo SUPAIDEAS/passifypdf --title "Refactoring: Use standard logging module instead of print" --body "All console output is currently using print(). We should initialize a basic logging configuration and use logger.info(), logger.error() instead." --label "good first issue" +gh issue create --repo SUPAIDEAS/passifypdf --title "CI: Add pre-commit hooks for code formatting" --body "To maintain code quality, we should add a .pre-commit-config.yaml file configuring ruff, black, or flake8 and add instructions on how to set it up." --label "good first issue" +gh issue create --repo SUPAIDEAS/passifypdf --title "Tests: Add type hints to all test fixtures and functions" --body "The main codebase uses type hints nicely, but the test suite (tests/unittests/test_encryptpdf.py) lacks them. This is a great starting issue to get familiar with the codebase." --label "good first issue" + +echo "Creating Help Wanted issues..." +# Help Wanted +gh issue create --repo SUPAIDEAS/passifypdf --title "Research: Implement hardware-accelerated AES encryption" --body "For large PDFs, standard pypdf encryption might be slow. We are looking for help to profile and possibly integrate hardware-accelerated AES via cryptography or natively." --label "help wanted" +gh issue create --repo SUPAIDEAS/passifypdf --title "Build: Implement cross-platform native binaries using PyInstaller" --body "Not all users have Python installed. Creating a GitHub Actions pipeline to publish standalone binaries for Windows, macOS, and Linux using PyInstaller would be a fantastic addition." --label "help wanted" +gh issue create --repo SUPAIDEAS/passifypdf --title "UX: Create a Streamlit or Gradio based web UI wrapper" --body "Some users prefer graphical interfaces. We need help building a simple, local Web UI wrapper around passifypdf using Streamlit or Gradio." --label "help wanted" +gh issue create --repo SUPAIDEAS/passifypdf --title "UX: Integrate into right-click OS context menus" --body "It would be amazing if users could right-click a PDF on Windows/macOS and select 'Protect with passifypdf'. We need contributors familiar with OS registries/automator to add this." --label "help wanted" +gh issue create --repo SUPAIDEAS/passifypdf --title "Benchmarking: Compare encryption speeds against other CLI tools" --body "We want to know how passifypdf performs against tools like qpdf or pdftk. We need help writing a benchmark suite and documenting the results." --label "help wanted" + diff --git a/passifypdf/encryptpdf.py b/passifypdf/encryptpdf.py index 13de80c..e74f855 100644 --- a/passifypdf/encryptpdf.py +++ b/passifypdf/encryptpdf.py @@ -61,7 +61,7 @@ def main() -> int: try: output_path = Path(args.output) - if output_path.exists() and not getattr(args, 'force', False): + if output_path.exists() and not args.force: response = input(f"File '{args.output}' already exists. Overwrite? [y/N]: ") if response.lower() not in ('y', 'yes'): print("Operation cancelled.") diff --git a/tests/unittests/test_cli.py b/tests/unittests/test_cli.py new file mode 100644 index 0000000..d2cb369 --- /dev/null +++ b/tests/unittests/test_cli.py @@ -0,0 +1,42 @@ +import argparse +from unittest import TestCase +from unittest.mock import patch +from passifypdf.cli import get_arg_parser + +class TestCli(TestCase): + """Unit tests for the cli module.""" + + @patch("passifypdf.cli.version") + def test_get_arg_parser_with_version(self, mock_version): + """Test argument parser when package version is found.""" + mock_version.return_value = "1.2.3" + parser = get_arg_parser() + + # We can test if the --version flag is handled correctly by capturing exit and stdout + with patch('sys.stdout') as mock_stdout: + with self.assertRaises(SystemExit) as cm: + parser.parse_args(["--version"]) + self.assertEqual(cm.exception.code, 0) + + # Unfortunately, argparse action='version' prints directly to stdout/sys.stdout. + # So we can't easily assert on the exact printed string here if argparse handles it natively, + # but we can verify the version is added to the format string. + # As an alternative, we know version is used inside get_arg_parser: + # arg_parser.add_argument("-v", "--version", action="version", version=f"%(prog)s {__version__}") + has_version_action = any(action.dest == 'version' for action in parser._actions) + self.assertTrue(has_version_action) + + @patch("passifypdf.cli.version") + def test_get_arg_parser_without_version(self, mock_version): + """Test argument parser when package is not installed (PackageNotFoundError).""" + from importlib.metadata import PackageNotFoundError + mock_version.side_effect = PackageNotFoundError() + parser = get_arg_parser() + + with patch('sys.stdout') as mock_stdout: + with self.assertRaises(SystemExit) as cm: + parser.parse_args(["--version"]) + self.assertEqual(cm.exception.code, 0) + + has_version_action = any(action.dest == 'version' for action in parser._actions) + self.assertTrue(has_version_action) diff --git a/tests/unittests/test_encryptpdf.py b/tests/unittests/test_encryptpdf.py index 61b2556..f9ab4b5 100644 --- a/tests/unittests/test_encryptpdf.py +++ b/tests/unittests/test_encryptpdf.py @@ -2,6 +2,7 @@ from unittest.mock import patch, mock_open from passifypdf.encryptpdf import encrypt_pdf import os +import shutil import tempfile from pypdf import PdfReader @@ -25,7 +26,7 @@ def tearDown(self): if os.path.exists(self.output_pdf): os.remove(self.output_pdf) if os.path.exists(self.temp_dir): - os.rmdir(self.temp_dir) + shutil.rmtree(self.temp_dir) def test_encrypt_pdf_creates_output_file(self): """Test that encrypt_pdf creates an output file.""" @@ -160,6 +161,7 @@ def test_main_with_force_flag(self, mock_print, mock_path_cls, mock_encrypt_pdf, self.assertEqual(result, 0) mock_encrypt_pdf.assert_called_with('in.pdf', 'out.pdf', 'pass') + mock_path_cls.assert_called_with('out.pdf') @patch('passifypdf.encryptpdf.get_arg_parser') @patch('passifypdf.encryptpdf.encrypt_pdf') @@ -182,6 +184,7 @@ def test_main_without_force_flag_user_says_yes(self, mock_input, mock_print, moc self.assertEqual(result, 0) mock_encrypt_pdf.assert_called_with('in.pdf', 'out.pdf', 'pass') + mock_path_cls.assert_called_with('out.pdf') @patch('passifypdf.encryptpdf.get_arg_parser') @patch('passifypdf.encryptpdf.encrypt_pdf') @@ -205,3 +208,26 @@ def test_main_without_force_flag_user_says_no(self, mock_input, mock_print, mock self.assertEqual(result, 0) mock_encrypt_pdf.assert_not_called() mock_print.assert_called_with("Operation cancelled.") + mock_path_cls.assert_called_with('out.pdf') + + @patch('passifypdf.encryptpdf.get_arg_parser') + @patch('passifypdf.encryptpdf.encrypt_pdf') + @patch('passifypdf.encryptpdf.Path') + @patch('builtins.print') + @patch('builtins.input') + def test_main_without_force_flag_file_not_exists(self, mock_input, mock_print, mock_path_cls, mock_encrypt_pdf, mock_arg_parser): + """Test main() without --force flag when output file does not exist.""" + mock_parser_instance = mock_arg_parser.return_value + mock_args = type('Args', (), {'input': 'in.pdf', 'output': 'out.pdf', 'passwd': 'pass', 'force': False})() + mock_parser_instance.parse_args.return_value = mock_args + + mock_path_instance = mock_path_cls.return_value + mock_path_instance.exists.return_value = False + + from passifypdf.encryptpdf import main + result = main() + + self.assertEqual(result, 0) + mock_encrypt_pdf.assert_called_with('in.pdf', 'out.pdf', 'pass') + mock_path_cls.assert_called_with('out.pdf') + mock_input.assert_not_called() From ae391bf3d95d8fe229e03e66bdcad03ffd818627 Mon Sep 17 00:00:00 2001 From: riteshr19 Date: Fri, 20 Feb 2026 20:12:33 -0600 Subject: [PATCH 4/4] feat: address issues #36, #37, #40, #41, #42 - feat(tests): add -> None return type hints to all test methods and fixtures in test_encryptpdf.py and test_cli.py (closes #42) - feat(ci): add .pre-commit-config.yaml with ruff and pre-commit-hooks; update CONTRIBUTING.md with pre-commit setup instructions (closes #41) - refactor: replace print() calls with standard logging module in encryptpdf.py; update test mocks to patch logger instead of builtins.print (closes #40) - docs: add Exit Codes section and --force flag to docs/CLI_OPTIONS.md; fix outdated poetry references to use uv (closes #37) - docs: add Programmatic Usage (Python API) section to README.md; update install/usage instructions from poetry to uv; fix typos ('nay' -> 'any', 'complain' -> 'complaint', etc.) (closes #36) --- .pre-commit-config.yaml | 17 ++++++++++++ CONTRIBUTING.md | 23 ++++++++++++++++ README.md | 43 +++++++++++++++++++++--------- docs/CLI_OPTIONS.md | 13 ++++++++- passifypdf/encryptpdf.py | 12 ++++++--- tests/unittests/test_cli.py | 12 ++++----- tests/unittests/test_encryptpdf.py | 39 ++++++++++++++------------- 7 files changed, 118 insertions(+), 41 deletions(-) create mode 100644 .pre-commit-config.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..bd63f75 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,17 @@ +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.6.0 + hooks: + - id: trailing-whitespace + - id: end-of-file-fixer + - id: check-yaml + - id: check-added-large-files + + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.4.4 + hooks: + # Run the ruff linter + - id: ruff + args: [--fix] + # Run the ruff formatter + - id: ruff-format diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 017db4b..ce721fe 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -41,6 +41,29 @@ uv run pytest --cov=passifypdf ``` Please make sure all existing tests pass and write new tests for your newly added code before submitting a Pull Request. +## Code Quality + +### Pre-commit Hooks + +This project uses [`pre-commit`](https://pre-commit.com/) to enforce code quality automatically before each commit. The configuration lives in [`.pre-commit-config.yaml`](.pre-commit-config.yaml) and runs `ruff` (linter + formatter) plus standard file hygiene checks. + +To set it up locally: + +```bash +# Install pre-commit (using uv) +uv run pre-commit install + +# Or using pip +pip install pre-commit +pre-commit install +``` + +Once installed, hooks run automatically on `git commit`. To run them manually across all files: + +```bash +uv run pre-commit run --all-files +``` + ## Code Constraints and Formatting Currently, the project strives for PEP 8 compliance. While we may add automated pre-commit formatting hooks in the future, please try to: diff --git a/README.md b/README.md index ea88e13..f43d29a 100644 --- a/README.md +++ b/README.md @@ -13,25 +13,25 @@ git clone https://github.com/SUPAIDEAS/passifypdf.git ``` ## Install Dependencies -Uses [Poetry](https://python-poetry.org/) for dependency management. +Uses [`uv`](https://github.com/astral-sh/uv) for dependency management. ```bash cd passifypdf -pip install poetry -poetry install +curl -LsSf https://astral.sh/uv/install.sh | sh +uv sync ``` ## Usage -Run the CLI tool using `poetry run`: +Run the CLI tool using `uv run`: ```bash -poetry run passifypdf --help +uv run passifypdf --help ``` -Or activate the shell: +Or install into a virtual environment and use directly: ```bash -poetry shell -passifypdf --help +uv sync +.venv/bin/passifypdf --help ``` @@ -45,12 +45,31 @@ passifypdf -i input.pdf -o protected.pdf -p mySecretPassword ``` ## Known Issues -If you have nay special chars(example: an emoji like Star 🌟) in the PDF file, it gives a minor complain during execution. -But it still does the job, so you can ingore that "char or object error" which you see in the output. +If you have any special chars (example: an emoji like Star 🌟) in the PDF file, it gives a minor complaint during execution. +But it still does the job, so you can ignore that "char or object error" which you see in the output. + +## Programmatic Usage (Python API) + +You can also use `passifypdf` directly from your Python scripts: + +```python +from passifypdf.encryptpdf import encrypt_pdf + +# Encrypt a PDF file with a password +encrypt_pdf( + input_pdf="path/to/input.pdf", + output_pdf="path/to/protected.pdf", + password="mySecretPassword", +) +``` + +The `encrypt_pdf` function raises `FileNotFoundError` if the input file does not exist, +`IsADirectoryError` if the path points to a directory, and `Exception` for other +encryption failures. ## Note: -In general you can use passifypdf to protect your PDF files against chance attackers. -But you should not rely on this for mission-critical data or situation. +In general you can use passifypdf to protect your PDF files against chance attackers. +But you should not rely on this for mission-critical data or situations. ## Build & Run Locally Visit [BUILD.md](BUILD.md) \ No newline at end of file diff --git a/docs/CLI_OPTIONS.md b/docs/CLI_OPTIONS.md index 6442abc..9750846 100644 --- a/docs/CLI_OPTIONS.md +++ b/docs/CLI_OPTIONS.md @@ -7,12 +7,23 @@ The `passifypdf` tool accepts the following command-line arguments: | `-i` | `--input` | Yes | Path to the input PDF file. | | `-o` | `--output` | Yes | Path to the output (encrypted) PDF file. | | `-p` | `--passwd` | Yes | Password to use for encryption. **Avoid passing sensitive passwords directly on the command line, as they may be exposed via process listings and shell history.** | +| `-f` | `--force` | No | Overwrite the output file without prompting if it already exists. | | `-v` | `--version` | No | Show the program's version number and exit. | | `-h` | `--help` | No | Show the help message and exit. | ## Example Usage ```bash -# Using the installed CLI command (after `poetry install`) +# Using the installed CLI command passifypdf -i input.pdf -o protected.pdf -p mySecretPassword + +# Or via uv run (useful during local development) +uv run passifypdf -i input.pdf -o protected.pdf -p mySecretPassword ``` + +## Exit Codes + +| Code | Meaning | +| :--- | :--- | +| `0` | Success — the PDF was encrypted and saved successfully, or the user cancelled the operation. | +| `1` | Failure — an error occurred (e.g., input file not found, I/O error, encryption failure). | diff --git a/passifypdf/encryptpdf.py b/passifypdf/encryptpdf.py index e74f855..dc75621 100644 --- a/passifypdf/encryptpdf.py +++ b/passifypdf/encryptpdf.py @@ -1,5 +1,6 @@ """Core PDF encryption module.""" +import logging import sys from pathlib import Path from typing import Union @@ -8,6 +9,8 @@ from .cli import get_arg_parser +logger = logging.getLogger(__name__) + def encrypt_pdf(input_pdf: Union[str, Path], output_pdf: Union[str, Path], password: str) -> None: """ @@ -64,14 +67,17 @@ def main() -> int: if output_path.exists() and not args.force: response = input(f"File '{args.output}' already exists. Overwrite? [y/N]: ") if response.lower() not in ('y', 'yes'): - print("Operation cancelled.") + logger.info("Operation cancelled.") return 0 encrypt_pdf(args.input, args.output, args.passwd) - print(f"Congratulations!\nPDF file encrypted successfully and saved as '{args.output}'") + logger.info( + "Congratulations!\nPDF file encrypted successfully and saved as '%s'", + args.output, + ) return 0 except Exception as e: - print(f"Error: {e}", file=sys.stderr) + logger.error("Error: %s", e) return 1 diff --git a/tests/unittests/test_cli.py b/tests/unittests/test_cli.py index d2cb369..b332957 100644 --- a/tests/unittests/test_cli.py +++ b/tests/unittests/test_cli.py @@ -7,17 +7,17 @@ class TestCli(TestCase): """Unit tests for the cli module.""" @patch("passifypdf.cli.version") - def test_get_arg_parser_with_version(self, mock_version): + def test_get_arg_parser_with_version(self, mock_version: patch) -> None: """Test argument parser when package version is found.""" mock_version.return_value = "1.2.3" parser = get_arg_parser() - + # We can test if the --version flag is handled correctly by capturing exit and stdout with patch('sys.stdout') as mock_stdout: with self.assertRaises(SystemExit) as cm: parser.parse_args(["--version"]) self.assertEqual(cm.exception.code, 0) - + # Unfortunately, argparse action='version' prints directly to stdout/sys.stdout. # So we can't easily assert on the exact printed string here if argparse handles it natively, # but we can verify the version is added to the format string. @@ -27,16 +27,16 @@ def test_get_arg_parser_with_version(self, mock_version): self.assertTrue(has_version_action) @patch("passifypdf.cli.version") - def test_get_arg_parser_without_version(self, mock_version): + def test_get_arg_parser_without_version(self, mock_version: patch) -> None: """Test argument parser when package is not installed (PackageNotFoundError).""" from importlib.metadata import PackageNotFoundError mock_version.side_effect = PackageNotFoundError() parser = get_arg_parser() - + with patch('sys.stdout') as mock_stdout: with self.assertRaises(SystemExit) as cm: parser.parse_args(["--version"]) self.assertEqual(cm.exception.code, 0) - + has_version_action = any(action.dest == 'version' for action in parser._actions) self.assertTrue(has_version_action) diff --git a/tests/unittests/test_encryptpdf.py b/tests/unittests/test_encryptpdf.py index f9ab4b5..c4ba758 100644 --- a/tests/unittests/test_encryptpdf.py +++ b/tests/unittests/test_encryptpdf.py @@ -4,12 +4,13 @@ import os import shutil import tempfile +from typing import Generator from pypdf import PdfReader class TestEncryptPdf(TestCase): """Unit tests for the encrypt_pdf function.""" - def setUp(self): + def setUp(self) -> None: """Set up test fixtures.""" self.test_password = "test_password_123" self.temp_dir = tempfile.mkdtemp() @@ -21,14 +22,14 @@ def setUp(self): ) self.output_pdf = os.path.join(self.temp_dir, "encrypted_output.pdf") - def tearDown(self): + def tearDown(self) -> None: """Clean up test files.""" if os.path.exists(self.output_pdf): os.remove(self.output_pdf) if os.path.exists(self.temp_dir): shutil.rmtree(self.temp_dir) - def test_encrypt_pdf_creates_output_file(self): + def test_encrypt_pdf_creates_output_file(self) -> None: """Test that encrypt_pdf creates an output file.""" encrypt_pdf(self.input_pdf, self.output_pdf, self.test_password) self.assertTrue( @@ -36,7 +37,7 @@ def test_encrypt_pdf_creates_output_file(self): "Encrypted PDF file should be created" ) - def test_encrypt_pdf_output_is_encrypted(self): + def test_encrypt_pdf_output_is_encrypted(self) -> None: """Test that the output PDF is actually encrypted.""" encrypt_pdf(self.input_pdf, self.output_pdf, self.test_password) @@ -47,7 +48,7 @@ def test_encrypt_pdf_output_is_encrypted(self): "Output PDF should be encrypted" ) - def test_encrypt_pdf_with_correct_password(self): + def test_encrypt_pdf_with_correct_password(self) -> None: """Test that encrypted PDF can be decrypted with correct password.""" encrypt_pdf(self.input_pdf, self.output_pdf, self.test_password) @@ -61,7 +62,7 @@ def test_encrypt_pdf_with_correct_password(self): "Should be able to decrypt with correct password" ) - def test_encrypt_pdf_preserves_page_count(self): + def test_encrypt_pdf_preserves_page_count(self) -> None: """Test that encryption preserves the number of pages.""" # Read original page count original_reader = PdfReader(self.input_pdf) @@ -81,7 +82,7 @@ def test_encrypt_pdf_preserves_page_count(self): "Page count should be preserved after encryption" ) - def test_encrypt_pdf_with_invalid_input(self): + def test_encrypt_pdf_with_invalid_input(self) -> None: """Test encrypt_pdf with non-existent input file.""" non_existent_file = "non_existent_file.pdf" @@ -92,7 +93,7 @@ def test_encrypt_pdf_with_invalid_input(self): @patch('passifypdf.encryptpdf.PdfWriter') @patch('passifypdf.encryptpdf.Path') # Mock Path @patch('builtins.open', new_callable=mock_open) - def test_encrypt_pdf(self, mock_file, mock_path_cls, mock_writer_cls, mock_reader_cls): + def test_encrypt_pdf(self, mock_file: mock_open, mock_path_cls: patch, mock_writer_cls: patch, mock_reader_cls: patch) -> None: # Setup mocks mock_path_instance = mock_path_cls.return_value mock_path_instance.exists.return_value = True @@ -127,7 +128,7 @@ def test_encrypt_pdf(self, mock_file, mock_path_cls, mock_writer_cls, mock_reade mock_writer_instance.write.assert_called_with(mock_file()) @patch('passifypdf.encryptpdf.Path') - def test_encrypt_pdf_file_not_found(self, mock_path_cls): + def test_encrypt_pdf_file_not_found(self, mock_path_cls: patch) -> None: mock_path_instance = mock_path_cls.return_value mock_path_instance.exists.return_value = False @@ -135,7 +136,7 @@ def test_encrypt_pdf_file_not_found(self, mock_path_cls): encrypt_pdf("nonexistent.pdf", "output.pdf", "secret") @patch('passifypdf.encryptpdf.Path') - def test_encrypt_pdf_is_directory(self, mock_path_cls): + def test_encrypt_pdf_is_directory(self, mock_path_cls: patch) -> None: mock_path_instance = mock_path_cls.return_value mock_path_instance.exists.return_value = True mock_path_instance.is_file.return_value = False @@ -146,8 +147,8 @@ def test_encrypt_pdf_is_directory(self, mock_path_cls): @patch('passifypdf.encryptpdf.get_arg_parser') @patch('passifypdf.encryptpdf.encrypt_pdf') @patch('passifypdf.encryptpdf.Path') - @patch('builtins.print') - def test_main_with_force_flag(self, mock_print, mock_path_cls, mock_encrypt_pdf, mock_arg_parser): + @patch('passifypdf.encryptpdf.logger') + def test_main_with_force_flag(self, mock_logger: patch, mock_path_cls: patch, mock_encrypt_pdf: patch, mock_arg_parser: patch) -> None: """Test main() with --force flag when output file exists.""" mock_parser_instance = mock_arg_parser.return_value mock_args = type('Args', (), {'input': 'in.pdf', 'output': 'out.pdf', 'passwd': 'pass', 'force': True})() @@ -166,9 +167,9 @@ def test_main_with_force_flag(self, mock_print, mock_path_cls, mock_encrypt_pdf, @patch('passifypdf.encryptpdf.get_arg_parser') @patch('passifypdf.encryptpdf.encrypt_pdf') @patch('passifypdf.encryptpdf.Path') - @patch('builtins.print') + @patch('passifypdf.encryptpdf.logger') @patch('builtins.input') - def test_main_without_force_flag_user_says_yes(self, mock_input, mock_print, mock_path_cls, mock_encrypt_pdf, mock_arg_parser): + def test_main_without_force_flag_user_says_yes(self, mock_input: patch, mock_logger: patch, mock_path_cls: patch, mock_encrypt_pdf: patch, mock_arg_parser: patch) -> None: """Test main() without --force flag, user agrees to overwrite.""" mock_parser_instance = mock_arg_parser.return_value mock_args = type('Args', (), {'input': 'in.pdf', 'output': 'out.pdf', 'passwd': 'pass', 'force': False})() @@ -189,9 +190,9 @@ def test_main_without_force_flag_user_says_yes(self, mock_input, mock_print, moc @patch('passifypdf.encryptpdf.get_arg_parser') @patch('passifypdf.encryptpdf.encrypt_pdf') @patch('passifypdf.encryptpdf.Path') - @patch('builtins.print') + @patch('passifypdf.encryptpdf.logger') @patch('builtins.input') - def test_main_without_force_flag_user_says_no(self, mock_input, mock_print, mock_path_cls, mock_encrypt_pdf, mock_arg_parser): + def test_main_without_force_flag_user_says_no(self, mock_input: patch, mock_logger: patch, mock_path_cls: patch, mock_encrypt_pdf: patch, mock_arg_parser: patch) -> None: """Test main() without --force flag, user refuses to overwrite.""" mock_parser_instance = mock_arg_parser.return_value mock_args = type('Args', (), {'input': 'in.pdf', 'output': 'out.pdf', 'passwd': 'pass', 'force': False})() @@ -207,15 +208,15 @@ def test_main_without_force_flag_user_says_no(self, mock_input, mock_print, mock self.assertEqual(result, 0) mock_encrypt_pdf.assert_not_called() - mock_print.assert_called_with("Operation cancelled.") + mock_logger.info.assert_called_with("Operation cancelled.") mock_path_cls.assert_called_with('out.pdf') @patch('passifypdf.encryptpdf.get_arg_parser') @patch('passifypdf.encryptpdf.encrypt_pdf') @patch('passifypdf.encryptpdf.Path') - @patch('builtins.print') + @patch('passifypdf.encryptpdf.logger') @patch('builtins.input') - def test_main_without_force_flag_file_not_exists(self, mock_input, mock_print, mock_path_cls, mock_encrypt_pdf, mock_arg_parser): + def test_main_without_force_flag_file_not_exists(self, mock_input: patch, mock_logger: patch, mock_path_cls: patch, mock_encrypt_pdf: patch, mock_arg_parser: patch) -> None: """Test main() without --force flag when output file does not exist.""" mock_parser_instance = mock_arg_parser.return_value mock_args = type('Args', (), {'input': 'in.pdf', 'output': 'out.pdf', 'passwd': 'pass', 'force': False})()