Skip to content

Refactor item errors#484

Merged
NGTmeaty merged 1 commit intointernetarchive:mainfrom
vbanos:item-errors
Sep 19, 2025
Merged

Refactor item errors#484
NGTmeaty merged 1 commit intointernetarchive:mainfrom
vbanos:item-errors

Conversation

@vbanos
Copy link
Copy Markdown
Collaborator

@vbanos vbanos commented Sep 19, 2025

Previously, when we had an item error we returned fmt.Errorf("item is fresh but has children"). and then in the unit tests we had to use
expected: errors.New("item is fresh but has children"

There is no need to define an error multiple times. We define:

ErrItemFreshHasChildren = errors.New("item is fresh but has children")

and then use ErrItemFreshHasChildren everywhere.

This way, we can easily change the error text in one place and avoid breaking the unit tests.

Also, we could avoid doing error string comparison and use errors.Is but we can't do it right now because the tests have some special error cases and its not straightforward.

Previously, when we had an item error we returned `fmt.Errorf("item is fresh but has children")`.
and then in the unit tests we had to use
`expected: errors.New("item is fresh but has children"`

There is no need to define an error multiple times. We define:
```
ErrItemFreshHasChildren = errors.New("item is fresh but has children")
```
and then use `ErrItemFreshHasChildren` everywhere.

This way, we can easily change the error text in one place and avoid
breaking the unit tests.

Also, we could avoid doing error string comparison and use `errors.Is`
but we can't do it right now because the tests have some special error
cases and its not straightforward.
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 56.33%. Comparing base (0837f4d) to head (351fa5b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/models/item.go 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #484      +/-   ##
==========================================
- Coverage   56.42%   56.33%   -0.09%     
==========================================
  Files         130      130              
  Lines        8091     8091              
==========================================
- Hits         4565     4558       -7     
- Misses       3161     3167       +6     
- Partials      365      366       +1     
Flag Coverage Δ
e2etests 40.61% <0.00%> (-0.04%) ⬇️
unittests 29.33% <87.50%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

Copy link
Copy Markdown
Collaborator

@NGTmeaty NGTmeaty left a comment

Choose a reason for hiding this comment

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

Thank you!

@NGTmeaty NGTmeaty merged commit 6d0b29e into internetarchive:main Sep 19, 2025
2 checks passed
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.

3 participants