Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(tests): properly test negative indexing #190

Merged
merged 2 commits into from
Jan 25, 2024

Conversation

hannahhoward
Copy link
Contributor

Goals

Fix #189

Implementation

Insure entity-bytes negative index test properly verifies that the last block of the underlying file is not read.

for the entity-bytes negative indexing test, fix the test so that it actually tests that the last
block in the file is not read
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Triage note for January: we may need to revisit this based on the result of discussion in ipfs/boxo#523 (comment)

@@ -634,7 +634,7 @@ func TestTrustlessCarEntityBytes(t *testing.T) {
HasBlocks(
flattenStrings(t,
subdirWithMixedBlockFiles.MustGetCid("subdir", "multiblock.txt"),
subdirWithMixedBlockFiles.MustGetDescendantsCids("subdir", "multiblock.txt")[:5])...,
subdirWithMixedBlockFiles.MustGetDescendantsCids("subdir", "multiblock.txt")[:len(subdirWithMixedBlockFiles.MustGetDescendantsCids("subdir", "multiblock.txt"))-1])...,
Copy link
Member

Choose a reason for hiding this comment

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

#189 highlights the instance of problem flagged in #112

we should avoid generating code in cases like this, where we have small number of CIDs, just write them inline. The list is small enough to not pollute test case, and makes it immune to boxo bugs/regressions.

Copy link
Member

Choose a reason for hiding this comment

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

Wrote some thoughts in #112 (comment), but that work is separate from fix here, not a blocker.

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Fixed minor bugs and tested end-to-end in ipfs/kubo#10320, where CI is green.

Merging, will release as 0.5.x later this week (incl. bubbling up to Kubo/Boxo).

@lidel lidel merged commit dc25ca6 into ipfs:main Jan 25, 2024
11 of 13 checks passed
@github-actions github-actions bot mentioned this pull request Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
2 participants