Skip to content

Use pytest.mark.skip for ChemRxiv tests instead of comments#86

Merged
njzjz merged 5 commits into
skip-chemrxivfrom
copilot/sub-pr-85
Feb 1, 2026
Merged

Use pytest.mark.skip for ChemRxiv tests instead of comments#86
njzjz merged 5 commits into
skip-chemrxivfrom
copilot/sub-pr-85

Conversation

Copilot AI commented Feb 1, 2026

Copy link
Copy Markdown
Contributor
  • Add optional skip_reason field to ReferenceCase dataclass
  • Uncomment the ChemRxiv test case and add skip_reason
  • Update test_output.py to use pytest.param with pytest.mark.skip for skipped cases
  • Update test_from_identifier.py to use pytest.param with pytest.mark.skip for skipped cases
  • Validate changes by running tests
  • Refactor to reduce code duplication with helper functions
  • Fix mypy type error in test_from_identifier.py
  • Final review

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com>
@pull-request-size pull-request-size Bot added size/L and removed size/XS labels Feb 1, 2026
Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com>

Copilot AI commented Feb 1, 2026

Copy link
Copy Markdown
Contributor Author

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • eutils.ncbi.nlm.nih.gov
    • Triggering command: /usr/bin/python python -m pytest tests/test_from_identifier.py -v (dns block)
  • export.arxiv.org
    • Triggering command: /usr/bin/python python -m pytest tests/test_from_identifier.py -v (dns block)
  • www.ncbi.nlm.nih.gov
    • Triggering command: /usr/bin/python python -m pytest tests/test_from_identifier.py -v (dns block)
    • Triggering command: /usr/bin/python /usr/bin/python -m wenxian from 10.1021/acs.jctc.2c01172 (dns block)
    • Triggering command: /usr/bin/python /usr/bin/python -m wenxian from 10.1021/acs.jctc.2c01172 -o /tmp/tmpcjwsx99m (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Address feedback on skipping ChemRxiv tests in PR #85 Use pytest.mark.skip for ChemRxiv tests instead of comments Feb 1, 2026
Copilot AI requested a review from njzjz February 1, 2026 16:36
@codecov

codecov Bot commented Feb 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.59%. Comparing base (4484383) to head (41eb096).
⚠️ Report is 6 commits behind head on skip-chemrxiv.

Additional details and impacted files
@@              Coverage Diff               @@
##           skip-chemrxiv      #86   +/-   ##
==============================================
  Coverage          88.59%   88.59%           
==============================================
  Files                 13       13           
  Lines                526      526           
==============================================
  Hits                 466      466           
  Misses                60       60           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@njzjz

njzjz commented Feb 1, 2026

Copy link
Copy Markdown
Owner

pre-commit.ci autofix

@njzjz

njzjz commented Feb 1, 2026

Copy link
Copy Markdown
Owner

@copilot mypy.....................................................................Failed

  • hook id: mypy
  • exit code: 1

tests/test_from_identifier.py:27: error: Argument 2 to "_create_test_param" has incompatible type "str | None"; expected "str" [arg-type]
Found 1 error in 1 file (checked 21 source files

Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com>

Copilot AI commented Feb 1, 2026

Copy link
Copy Markdown
Contributor Author

@copilot mypy.....................................................................Failed

  • hook id: mypy
  • exit code: 1...

Fixed in 41eb096. Changed the identifier parameter type from str to str | None to match the actual types of doi and arxiv fields which are optional.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • www.ncbi.nlm.nih.gov
    • Triggering command: /usr/bin/python python -m pytest tests/test_from_identifier.py::test_from_identifier -v --tb=short (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@njzjz njzjz marked this pull request as ready for review February 1, 2026 19:58
Copilot AI review requested due to automatic review settings February 1, 2026 19:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the test suite to use pytest's built-in skip mechanism instead of commenting out problematic test cases. It enables the previously commented ChemRxiv test case and marks it to be skipped with a clear reason.

Changes:

  • Added optional skip_reason field to the ReferenceCase dataclass
  • Uncommented the ChemRxiv test case and marked it with skip_reason="ChemRxiv API broken"
  • Introduced _create_test_param helper functions in both test files to reduce code duplication and properly handle test skipping

Reviewed changes

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

File Description
tests/cases.py Added skip_reason field to ReferenceCase dataclass and uncommented the ChemRxiv test case with skip reason
tests/test_output.py Added _create_test_param helper function and updated parametrize decorators to use pytest.param with skip marks
tests/test_from_identifier.py Added _create_test_param helper function and updated parametrize decorator to use pytest.param with skip marks

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@njzjz njzjz merged commit 3baaa03 into skip-chemrxiv Feb 1, 2026
18 checks passed
@njzjz njzjz deleted the copilot/sub-pr-85 branch February 1, 2026 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants