Skip to content

Test wds2idx names with spaces#6414

Open
JanuszL wants to merge 1 commit into
NVIDIA:mainfrom
JanuszL:test-wds2idx-space-filename
Open

Test wds2idx names with spaces#6414
JanuszL wants to merge 1 commit into
NVIDIA:mainfrom
JanuszL:test-wds2idx-space-filename

Conversation

@JanuszL

@JanuszL JanuszL commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Category:

Other

Description:

Extend the GNU tar regression test for wds2idx to cover a tar member name
containing both a non-ASCII character and spaces. This protects the positional
size parsing from regressions when filenames contain spaces.

Additional information:

Affected modules and functionalities:

  • dali/test/python/reader/test_wds2idx.py: GNU tar index generation
    regression coverage.

Key points relevant for the review:

The test uses my imgé file.jpg, ensuring the filename occupies the final
field of GNU tar's verbose output while the size remains parsed from field 2.

Tests:

  • Existing tests apply
  • New tests added
    • Python tests
    • GTests
    • Benchmark
    • Other
  • N/A

Checklist

Documentation

  • Existing documentation applies
  • Documentation updated
    • Docstring
    • Doxygen
    • RST
    • Jupyter
    • Other
  • N/A

DALI team only

Requirements

  • Implements new requirements
  • Affects existing requirements
  • N/A

REQ IDs: N/A

JIRA TASK: N/A

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
@greptile-apps

greptile-apps Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extends the existing GNU tar regression test for wds2idx index generation by changing the tar member filename from a pure non-ASCII name (imgé.jpg) to one that contains both spaces and a non-ASCII character (my imgé file.jpg). The test function is renamed to reflect the broader coverage.

  • The change guards the positional size-field parsing in _get_data_tar() (line 120: types_sizes_line.split(maxsplit=5)[2]) against regressions when filenames contain spaces, since a naïve split without maxsplit would misalign the size field.
  • Assertions verify both the parsed name and the parsed size against the concrete expected values, so the test would catch a regression that returns the wrong size or a truncated/misread name.

Confidence Score: 5/5

A minimal, well-scoped test change with no production-code modifications; safe to merge.

The only change is renaming one test function and widening the tar member filename to include spaces alongside the existing non-ASCII character. The assertions still verify both name and size against concrete expected values, so the test would catch any regression in the positional field parsing. No production code is touched, the skip guard for missing GNU tar is preserved, and the copyright year is current.

No files require special attention.

Important Files Changed

Filename Overview
dali/test/python/reader/test_wds2idx.py Regression test extended: member name changed from "imgé.jpg" to "my imgé file.jpg" to cover filenames with both spaces and non-ASCII characters; test function renamed accordingly. Assertions unchanged and still validate both name and size against concrete expected values.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Test as test function
    participant tarfile as tarfile (stdlib)
    participant IndexCreator as IndexCreator._get_data_tar()
    participant tar as GNU tar subprocess

    Test->>tarfile: create "data.tar" with member "my imgé file.jpg"
    tarfile-->>Test: archive written

    Test->>IndexCreator: _get_data_tar()
    IndexCreator->>tar: tar --list --block-num --file data.tar
    tar-->>IndexCreator: "block N: my imgé file.jpg"
    IndexCreator->>tar: tar --verbose --list --file data.tar
    tar-->>IndexCreator: "-rw... <size> <date> my imgé file.jpg"
    IndexCreator-->>Test: yield (offset, "my imgé file.jpg", size)

    Test->>Test: "assert name == "my imgé file.jpg""
    Test->>Test: "assert size == len(payload)"
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Test as test function
    participant tarfile as tarfile (stdlib)
    participant IndexCreator as IndexCreator._get_data_tar()
    participant tar as GNU tar subprocess

    Test->>tarfile: create "data.tar" with member "my imgé file.jpg"
    tarfile-->>Test: archive written

    Test->>IndexCreator: _get_data_tar()
    IndexCreator->>tar: tar --list --block-num --file data.tar
    tar-->>IndexCreator: "block N: my imgé file.jpg"
    IndexCreator->>tar: tar --verbose --list --file data.tar
    tar-->>IndexCreator: "-rw... <size> <date> my imgé file.jpg"
    IndexCreator-->>Test: yield (offset, "my imgé file.jpg", size)

    Test->>Test: "assert name == "my imgé file.jpg""
    Test->>Test: "assert size == len(payload)"
Loading

Reviews (1): Last reviewed commit: "Test wds2idx names with spaces" | Re-trigger Greptile

@JanuszL

JanuszL commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

!build

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [56341050]: BUILD STARTED

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [56341050]: BUILD PASSED

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants