[Merged by Bors] - Fix misleading error #4725
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #4725 +/- ##
=========================================
- Coverage 77.2% 76.9% -0.3%
=========================================
Files 255 255
Lines 28805 28844 +39
=========================================
- Hits 22246 22197 -49
- Misses 5171 5261 +90
+ Partials 1388 1386 -2
|
| }, { | ||
| name: "file empty", | ||
| data: []byte(""), | ||
| crc: []byte{0xC6, 0x10, 0x72, 0xDF, 0x52, 0xD7, 0x74, 0x0E}, | ||
| errMsg: "wrong checksum 0xC61072DF52D7740E, computed 0x0", | ||
| }, |
There was a problem hiding this comment.
I'm confused by this test. The CRC64 of an empty file should be 0x0000000000000000 and if the file only contains those bytes it will pass reading & CRC validation
There was a problem hiding this comment.
What is confusing? It's exactly as you said. CRC of nothing is 0. The test file contains a wrong CRC and no data hence it should fail with "wrong checksum 0xC61072DF52D7740E, computed 0x0" message.
There was a problem hiding this comment.
but isn't this just a repetition of the "badCRC" testcase?
There was a problem hiding this comment.
No, it's an edge case when the input is empty.
There was a problem hiding this comment.
Again back to my first comment: an empty input is not an error, it has a valid checksum. This test does not check if empty input is handled correctly (that would be "" and 0x0), it checks if a wrong checksum is detected (as does the "badCRC" testcase)
|
bors merge |
## Motivation Users often get confused by the log message when the nipost_challenge.bin doesn't exist. This is a normal situation, yet the log includes "errormsg". ## Changes Don't log an error in case the file is not found. Additionally, includes a fix for a bug discovered during testing. The code panicked when the file was too small (for example empty). ## Test Plan - UT - [x] manual test
|
Build failed: |
|
bors merge |
## Motivation Users often get confused by the log message when the nipost_challenge.bin doesn't exist. This is a normal situation, yet the log includes "errormsg". ## Changes Don't log an error in case the file is not found. Additionally, includes a fix for a bug discovered during testing. The code panicked when the file was too small (for example empty). ## Test Plan - UT - [x] manual test
|
Build failed: |
|
flake in bors merge |
## Motivation Users often get confused by the log message when the nipost_challenge.bin doesn't exist. This is a normal situation, yet the log includes "errormsg". ## Changes Don't log an error in case the file is not found. Additionally, includes a fix for a bug discovered during testing. The code panicked when the file was too small (for example empty). ## Test Plan - UT - [x] manual test
|
Build failed: |
|
bors merge |
## Motivation Users often get confused by the log message when the nipost_challenge.bin doesn't exist. This is a normal situation, yet the log includes "errormsg". ## Changes Don't log an error in case the file is not found. Additionally, includes a fix for a bug discovered during testing. The code panicked when the file was too small (for example empty). ## Test Plan - UT - [x] manual test
|
Pull request successfully merged into develop. Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
bors
bot
changed the title
Motivation
Users often get confused by the log message when the nipost_challenge.bin doesn't exist. This is a normal situation, yet the log includes "errormsg".
Changes
Don't log an error in case the file is not found.
Additionally, includes a fix for a bug discovered during testing. The code panicked when the file was too small (for example empty).
Test Plan