Skip to content

Conversation

@NakulSingh156
Copy link

@NakulSingh156 NakulSingh156 commented Jan 21, 2026

Pull Request

Description

I investigated Issue #24 ("deploy related -> content variant issue") where a specific URL format was reported to cause an IndexError.

I attempted to reproduce the crash using the current main branch, but the issue appears to be already resolved (likely by the existing safety check if len(args) < 2).

To ensure this remains fixed and to close the issue, I have added a regression test case in tests/test_deploy.py that uses the exact input reported in the issue.

Changes:
Added unit test case to test_get_content_variants verifying that a URL without variant segments returns {} instead of crashing.

Related Issues
Resolves #24.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update
  • Housekeeping

Checklist:

  • My code follows the ruff code style of this project.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
    • poetry run pytest - all tests passed
    • poetry run ruff check - no linting errors

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Fixed handling of URLs without content variants that previously caused errors.
  • Tests

    • Added regression tests for deployment scenarios.
    • Updated test expectations to align with current deployment payload structures.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 21, 2026

📝 Walkthrough

Walkthrough

This PR adds a regression test for handling URLs without content variants and updates test expectations following changes to the deployment API signature and response structure for artifact/version relationships.

Changes

Cohort / File(s) Summary
Regression test and test updates
tests/test_deploy.py
Added test_get_content_variants regression test to prevent IndexError when processing URLs with no content variants. Uncommented and adjusted test_distribution_cases and test_empty_cvs tests to match new get_file_info(dst_string) signature (removed artifact_name parameter) and updated expected JSON-LD payload structure with Artifact/distribution field changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • Refactor/project structure #39: Both PRs modify the deployment API by updating the get_file_info function signature from get_file_info(artifact_name, dst_string) to get_file_info(dst_string) and adjusting corresponding call sites in tests and implementation.
🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Out of Scope Changes check ❓ Inconclusive The PR summary indicates test modifications beyond the regression test (test_distribution_cases, test_empty_cvs), which appear unrelated to the stated objective of verifying Issue #24. Clarify whether the uncommented/modified tests are necessary for this PR or should be separated into a different pull request addressing Issue #8.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically identifies the main change: adding a regression test to verify a fix for Issue #24.
Description check ✅ Passed The description comprehensively covers the investigation, the issue context, the solution added, and includes all required template sections with appropriate checkboxes marked.
Linked Issues check ✅ Passed The PR adds a regression test that directly addresses Issue #24 by reproducing the exact input that caused an IndexError and verifying the fix returns an empty dict instead of crashing.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

deploy related -> content variant issue

1 participant