Create unit tests for pkg/tar-patch package

[melkoudi-cmyk]

Added unit tests for the tar-patch package
resulting in 82.9% coverage

THEEDGE-4594

Signed-off-by: Meriem Elkoudi melkoudi@redhat.com

Assisted by: Claude

[gemini-code-assist]

Code Review

This pull request adds a comprehensive suite of unit and integration tests in pkg/tar-patch/apply_test.go to verify the Apply function and the FilesystemDataSource implementation. Feedback on the changes highlights that the path traversal test (TestApply_PathTraversal) is currently a false positive because it uses an empty mock data source, which would fail on any file lookup regardless of whether path traversal was successfully blocked. It is recommended to refactor this test using a real filesystem data source and a file created outside the temporary directory to ensure path traversal attempts are robustly caught.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.43%. Comparing base (ec0536e) to head (f41af3c).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #103      +/-   ##
==========================================
+ Coverage   79.19%   81.43%   +2.24%     
==========================================
  Files          10       10              
  Lines        1115     1115              
==========================================
+ Hits          883      908      +25     
+ Misses        134      117      -17     
+ Partials       98       90       -8     

☔ View full report in Codecov by Harness.
📢 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.

[knecasov]

I have a few notes.

• Consider adding tests for operations (DeltaOpCopy, DeltaOpAddData, DeltaOpSeek) sent without a preceding DeltaOpOpen. This could reveal potential nil pointer issues.

• The mock has error injection fields (closeErr, openErr, readErr, seekErr) that are never set in any test (their default value should be nil). Consider adding tests that inject errors to verify that Apply() correctly propagates I/O failures from the data source (for example, permission denied on open, disk read failure during copy).

• I would recommend unifying the error message style in test assertions - some use quotes around the expected string (for example, 'no current file') and some do not.

I would consider adding the following edge cases.

• DeltaOpData / DeltaOpCopy with size: 0
• DeltaOpOpen with an empty filename (size: 0)
• seek past the end of a file followed by a copy

[djach7]

Thanks for working on this! I've got a couple change requests and a further question about error message style:

It looks to me like there are 3 different types of error message style here: single quotes, escaped double quotes, and no quotes. Would you mind updating this to use one consistent style? I'd recommend single quotes, I think that's what we use most often. Here's where I'm seeing the different styles:

Style 1: Single quotes 'error message'

• Line 308: expected 'invalid delta format' error
• Line 327: expected 'unexpected delta op' error
• Line 584: expected 'no current file' error
• Line 651: expected 'no current file' error

Style 2: Escaped double quotes "error message"

• Line 351: expected "filename size" error
• Line 376: expected "AddData" size error
• Line 791: expected "no current file" error
• Line 810: expected "no current file" error
• Line 828: expected "no current file" error
• Line 850: expected "permission denied" error
• Line 874: expected "disk read failure" error
• Line 898: expected "seek failed" error
• Line 923: expected "I/O error during read" error
• Line 984: expected "invalid source name" error

Style 3: No quotes

• Line 1009: expected EOF error

[djach7]

@melkoudi-cmyk Thanks for addressing my comments! It looks like the path traversal test still has some kinks to work out and the tests are failing on an unchecked error and if/else issue I think?

[knecasov]

LGTM, thank you very much for incorporating my comments!

Note: I looked that the same tests failed also in the previous PR #97, so it seems they are still expected to fail.

[melkoudi-cmyk]

LGTM, thank you very much for incorporating my comments!

Note: I looked that the same tests failed also in the previous PR #97, so it seems they are still expected to fail.

There is a bug within the test that tests the windows version. Rosy and I are trying to find a solution to this as well

[djach7]

LGTM! Looks like this just needs signatures on the commits and then it's all good