Skip to content

Overwriting redirects with an item is no longer allowed#1072

Merged
kelson42 merged 7 commits into
mainfrom
overwriting_redirects_with_an_item_no_longer_possible
Apr 18, 2026
Merged

Overwriting redirects with an item is no longer allowed#1072
kelson42 merged 7 commits into
mainfrom
overwriting_redirects_with_an_item_no_longer_possible

Conversation

@veloman-yunkan

Copy link
Copy Markdown
Collaborator

Fixes #1071

The PR mostly consists of new unit-tests demonstrating how attempts to overwrite existing entries work. Various combinations of the types of the existing entry and the entry being added are tested. The last commit eliminates the deliberate possibility of overwriting a redirection entry with an item entry, updating the unit tests correspondingly.

@codecov

codecov Bot commented Apr 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 56.48%. Comparing base (f730b85) to head (9374cb4).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
src/writer/creator.cpp 0.00% 0 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1072      +/-   ##
==========================================
+ Coverage   56.42%   56.48%   +0.06%     
==========================================
  Files         101      101              
  Lines        5069     5065       -4     
  Branches     2211     2208       -3     
==========================================
+ Hits         2860     2861       +1     
+ Misses        741      737       -4     
+ Partials     1468     1467       -1     

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

@veloman-yunkan veloman-yunkan requested a review from kelson42 April 18, 2026 13:01
@kelson42

kelson42 commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

@veloman-yunkan How does the libzim behaves if we try to overwrite an item?

I thought we should get rid of ASSERT!? #723

@veloman-yunkan

Copy link
Copy Markdown
Collaborator Author

@veloman-yunkan How does the libzim behaves if we try to overwrite an item?

A zim::InvalidEntry exception is thrown.

I thought we should get rid of ASSERT!? #723

How does it apply to this PR?

@kelson42 kelson42 merged commit 6667d66 into main Apr 18, 2026
26 of 27 checks passed
@kelson42 kelson42 deleted the overwriting_redirects_with_an_item_no_longer_possible branch April 18, 2026 17:37
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.

Should we allow overwriting a redirection with an item during ZIM creation?

2 participants