Skip to content

Add additional test coverage#1

Merged
jecsand838 merged 1 commit intojecsand838:avro-row-encoderfrom
alamb:alamb/tests
Jan 24, 2026
Merged

Add additional test coverage#1
jecsand838 merged 1 commit intojecsand838:avro-row-encoderfrom
alamb:alamb/tests

Conversation

@alamb
Copy link
Copy Markdown

@alamb alamb commented Jan 24, 2026

While reviewing apache#9171 from @jecsand838 I used code coverage cargo llvm-cov and found some uncovered code.

I wrote some additional tests with codex for your consideration

@jecsand838 jecsand838 merged commit 9891885 into jecsand838:avro-row-encoder Jan 24, 2026
22 of 23 checks passed
@jecsand838
Copy link
Copy Markdown
Owner

@alamb These look great! I can clean them up on my end. Thank you so much for helping with this 😄

jecsand838 added a commit that referenced this pull request Jan 24, 2026
Add additional test coverage
jecsand838 pushed a commit that referenced this pull request Feb 13, 2026
# Which issue does this PR close?

small optimization

# Rationale for this change
key insight is the byte clone is cheap just a ref count compare to vec
clone is a alloc + memcopy.

before
```
let mut result = Vec::new();          // alloc #1
result.extend_from_slice(prefix);
result.extend_from_slice(suffix);

let data = Bytes::from(result.clone()); // alloc apache#2 + memcpy
item.set_from_bytes(data);
self.previous_value = result;          // keep Vec
```

after
```
let mut result = Vec::with_capacity(prefix_len + suffix.len()); // alloc #1
result.extend_from_slice(&self.previous_value[..prefix_len]);
result.extend_from_slice(suffix);

let data = Bytes::from(result);       // no alloc, takes Vec buffer
item.set_from_bytes(data.clone());    // cheap refcount bump
self.previous_value = data;           // move, no alloc
```

# What changes are included in this PR?
previous_value type changed to Bytes
preallocate result vec capacity.

# Are these changes tested?

the existing test should pass

# Are there any user-facing changes?

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

2 participants