Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove xml_missing() tests #2397

Merged
merged 9 commits into from
Dec 8, 2023
Merged

Remove xml_missing() tests #2397

merged 9 commits into from
Dec 8, 2023

Conversation

IndrajeetPatil
Copy link
Collaborator

Closes #2359

@IndrajeetPatil IndrajeetPatil added internals Issues related to inner workings of lintr, i.e., not user-visible testing labels Dec 5, 2023
@codecov-commenter
Copy link

codecov-commenter commented Dec 5, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8029e1f) 98.68% compared to head (d824379) 98.64%.

❗ Current head d824379 differs from pull request most recent head 6fb1d51. Consider uploading reports for the commit 6fb1d51 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2397      +/-   ##
==========================================
- Coverage   98.68%   98.64%   -0.04%     
==========================================
  Files         126      126              
  Lines        5700     5610      -90     
==========================================
- Hits         5625     5534      -91     
- Misses         75       76       +1     

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

@AshesITR
Copy link
Collaborator

AshesITR commented Dec 5, 2023

The test should instead be refactored to honor is_linter_level(), I think.

@MichaelChirico
Copy link
Collaborator

I'll let @AshesITR give review here, thanks for the clean-up!

@IndrajeetPatil IndrajeetPatil removed the request for review from MichaelChirico December 6, 2023 09:07
@AshesITR
Copy link
Collaborator

AshesITR commented Dec 8, 2023

@IndrajeetPatil PTAL. I restored the test to a working one and removed the extra newlines left in the diff

AshesITR
AshesITR previously approved these changes Dec 8, 2023
AshesITR
AshesITR previously approved these changes Dec 8, 2023
@IndrajeetPatil
Copy link
Collaborator Author

Thanks, @AshesITR, for taking this to the finish line. I've made one minor change.

@AshesITR AshesITR merged commit 44db017 into main Dec 8, 2023
21 checks passed
@AshesITR AshesITR deleted the rm-xml-missing-tests branch December 8, 2023 18:21
@MichaelChirico
Copy link
Collaborator

This is beautiful 🥹

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internals Issues related to inner workings of lintr, i.e., not user-visible testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tests in get_source_expression() no longer relevant to actual lint() behavior.
4 participants