Skip to content

fix: Correct FileHash.compare to compare FileHash objects#13012

Open
andy-cave-nf wants to merge 2 commits into
dbt-labs:main_backupfrom
andy-cave-nf:file-hash-tests
Open

fix: Correct FileHash.compare to compare FileHash objects#13012
andy-cave-nf wants to merge 2 commits into
dbt-labs:main_backupfrom
andy-cave-nf:file-hash-tests

Conversation

@andy-cave-nf

@andy-cave-nf andy-cave-nf commented May 22, 2026

Copy link
Copy Markdown

Contributes to #8269

Problem

Hiya, first time contributing. I wanted to improve the unit test coverage and as I was adding to FileHash I noticed a bug in FileHash.compare. It compares FileHash objects with strings rather than other FileHash objects.

Solution

I've correct the comparison to be against FileHash objects rather than against the checksum attribute, which is a string.
I've also added extra tests to bring that file up to 100% coverage.

Checklist

  • I have read the contributing guide and understand what's expected of me.
  • I have run this code in development, and it appears to resolve the stated issue.
  • This PR includes tests, or tests are not required or relevant for this PR.
  • This PR has no interface changes (e.g., macros, CLI, logs, JSON artifacts, config files, adapter interface, etc.) or this PR has already received feedback and approval from Product or DX.
  • This PR includes type annotations for new and modified functions.

@andy-cave-nf andy-cave-nf requested a review from a team as a code owner May 22, 2026 14:34
@cla-bot

cla-bot Bot commented May 22, 2026

Copy link
Copy Markdown

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @andy-cave-nf

@github-actions

Copy link
Copy Markdown
Contributor

Additional Artifact Review Required

Changes to artifact directory files requires at least 2 approvals from core team members.

@github-actions github-actions Bot added the community This PR is from a community member label May 22, 2026

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gates Failed
Enforce advisory code health rules (1 file with Code Duplication)

Gates Passed
3 Quality Gates Passed

See analysis details in CodeScene

Reason for failure
Enforce advisory code health rules Violations Code Health Impact
test_base_resource.py 1 advisory rule 10.00 → 9.39 Suppress

Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

Comment thread tests/unit/artifacts/test_base_resource.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community This PR is from a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant