Issue #445. Testing the behavior of the strncasecmp_custom function.

[lecapitainenemo]

Related to issue #445 fix custom strncasecmp implementation.

It was described that the strncasecmp_custom function was going to fail and emit that strings like "not" and "NOTICE" were equal. However, the current implementation passed tests of this style (I added them).

This was a not a notable change so I did not add anything to ChangeLog.

Thanks.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.53%. Comparing base (72f40f9) to head (1fb7f34).
Report is 1 commits behind head on latest.

Additional details and impacted files

@@            Coverage Diff             @@
##           latest     #454      +/-   ##
==========================================
+ Coverage   90.48%   90.53%   +0.04%     
==========================================
  Files          47       47              
  Lines        4373     4374       +1     
  Branches      585      586       +1     
==========================================
+ Hits         3957     3960       +3     
+ Misses        281      280       -1     
+ Partials      135      134       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lecapitainenemo]

I was wondering how to proceed, if I committed a mistake or if there is any other recommended task.
Thank you

[goatshriek]

The bug can be subtle, but is present. By passing in a buffer size far greater than the string you provided, you've forced the comparison to go farther in and find the NULL character difference. However the from_buffer function needs to work when it doesn't have a NULL terminator. After all the purpose of this function is to NOT need a NULL terminator to begin with!

You can trigger the failure by reducing the buffer size to match the string length, add this test, or [my recommendation] do both.

TEST( GetSeverityEnum, InvalidButPrefix ) {
  enum stumpless_severity result = stumpless_get_severity_enum( "war" );
  EXPECT_EQ( result, -1 );
  EXPECT_ERROR_ID_EQ( STUMPLESS_INVALID_SEVERITY );
}

[lecapitainenemo]

Hey,
I added the failing tests. I removed the "EXPECT_ERROR_ID_EQ( STUMPLESS_INVALID_SEVERITY );" part from them, though. With the modification to strncasecmp_custom function, all the tests (new and old) pass. Please let me know if there is anything wrong. I appreciate your time and patience.

[goatshriek]

Ah, I didn't realize that error codes were not being raised. I've opened #455 to address that.

Everything looks good! One final ask: can you also add a new test for the opposite direction: that strings that start with a valid severity but are longer are not accepted. For example "warnings are neat" should not be parsed to a valid severity. This will make sure that the comparison works properly in both directions.

[lecapitainenemo]

The respective tests were added and all old and new passed.
Thank you

[goatshriek]

Thanks very much for putting this contribution together and sticking with it through the requested changes!