Skip to content

unittests: convert util-spm tests to FAIL/PASS API - v3#15368

Closed
0x-0ddc0de wants to merge 1 commit into
OISF:mainfrom
0x-0ddc0de:redmine-6334-v3
Closed

unittests: convert util-spm tests to FAIL/PASS API - v3#15368
0x-0ddc0de wants to merge 1 commit into
OISF:mainfrom
0x-0ddc0de:redmine-6334-v3

Conversation

@0x-0ddc0de
Copy link
Copy Markdown

Update to address comments on

Replaces conditionals within the loops with a single FAIL_IF.

Issue: 6334

@0x-0ddc0de 0x-0ddc0de requested a review from victorjulien as a code owner May 11, 2026 20:00
@jufajardini jufajardini mentioned this pull request May 11, 2026
Comment thread src/util-spm.c
Comment on lines 2359 to +2363
if (found == NULL) {
if (d->match_offset != SPM_NO_MATCH) {
printf(" should have matched at %" PRIu32 " but didn't\n",
d->match_offset);
ret = 0;
}
FAIL_IF(d->match_offset != SPM_NO_MATCH);
} else {
uint32_t offset = (uint32_t)(found - (const uint8_t *)d->haystack);
if (offset != d->match_offset) {
printf(" should have matched at %" PRIu32
" but matched at %" PRIu32 "\n",
d->match_offset, offset);
ret = 0;
}
FAIL_IF(offset != d->match_offset);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we find a way to also remove this if/ else case, following Victor's comment on the previous PR?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done

@jufajardini
Copy link
Copy Markdown
Contributor

jufajardini commented May 11, 2026

Thanks for following up on the feedback.

  • Approving the workflows to see if the failures from previous PR are still happening here.
  • added the CLA required for us to confirm that it was signed.

@jufajardini jufajardini changed the title unittests: convert util-spm tests to FAIL/PASS API unittests: convert util-spm tests to FAIL/PASS API - v3 May 11, 2026
@jufajardini jufajardini added the cla required The author has not yet signed the CLA or CLA signing is pending verification label May 11, 2026
@github-actions
Copy link
Copy Markdown

NOTE: This PR may contain new authors.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

Codecov Report

❌ Patch coverage is 95.45455% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.64%. Comparing base (72e3d7a) to head (6456514).
⚠️ Report is 25 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15368      +/-   ##
==========================================
- Coverage   82.66%   82.64%   -0.02%     
==========================================
  Files         993      996       +3     
  Lines      271004   271051      +47     
==========================================
- Hits       224022   224008      -14     
- Misses      46982    47043      +61     
Flag Coverage Δ
fuzzcorpus 61.03% <ø> (-0.04%) ⬇️
livemode 18.36% <ø> (-0.03%) ⬇️
netns 22.60% <ø> (-0.06%) ⬇️
pcap 45.16% <ø> (-0.13%) ⬇️
suricata-verify 66.36% <ø> (-0.05%) ⬇️
unittests 58.57% <95.45%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@inashivb
Copy link
Copy Markdown
Member

Replaced w #15371

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

Labels

cla required The author has not yet signed the CLA or CLA signing is pending verification

Development

Successfully merging this pull request may close these issues.

3 participants