8000 [DYOD] Group 1: increase test coverage by luisegarberding · Pull Request #2707 · hyrise/hyrise · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[DYOD] Group 1: increase test coverage #2707

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

[DYOD] Group 1: increase test coverage #2707

wants to merge 11 commits into from

Conversation

luisegarberding
Copy link

We improved the coverage of:

  1. variable_length_key
  2. variable_length_key_store
  3. fixed_string_vector

Coverage before changes:
image
pre
image

Coverage after:
image
post
image

luisegarberding and others added 4 commits June 2, 2025 14:33
@luisegarberding
Dyod group 1 luise (#2705)
ad5d89a 
increase coverage of `storage/fixed_string_dictionary_segment/fixed_string_vector.cpp`
@gwauge
DYOY Group 1: Georg (#2706)
c790de4 
increase coverage for VariableLengthKeyStore
@EnnioEnnio
add tests for variable_length_key.cpp (#2704)
d83b80b 
Based on Coverage Report Created: 2025-05-22 14:51, we are testing the
following previously uncovered member functions and operators:
1. VariableLengthKey::operator=(const VariableLengthKeyBase&)
2. VariableLengthKey::operator=(const VariableLengthKey&)
3. VariableLengthKey::operator!=(const VariableLengthKey&)
4. VariableLengthKey::operator!=(const VariableLengthKeyConstProxy&)
5. VariableLengthKey::bytes_per_key() const
6. operator<<(std::ostream&, const VariableLengthKey&)
@luisegarberding
use auto
b72777d
@EnnioEnnio EnnioEnnio added the FullCI Run all CI tests (slow, but required for merge) label Jun 4, 2025
dey4ss
dey4ss reviewed Jun 12, 2025
View reviewed changes
Copy link
Member
@dey4ss dey4ss left a comment

Choose a reason for hiding this comment

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

Nice tests overall! I just have a couple of comments.

src/test/lib/storage/index/group_key/variable_length_key_test.cpp Outdated
std::ostringstream oss;
oss << key;
const auto str = oss.str();
EXPECT_FALSE(str.empty());
Copy link
Member
@dey4ss dey4ss Jun 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I guess it would make sense to actually test the output and not only ensure it's non-empty.

Copy link

Choose a reason for hiding this comment

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

Check! I did it in the most basic way, maybe that's not enough, let me know

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

@dey4ss dey4ss dey4ss left review comments

@EnnioEnnio EnnioEnnio EnnioEnnio left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
FullCI Run all CI tests (slow, but required for merge)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@luisegarberding @dey4ss @EnnioEnnio @gwauge
0