Create test harness for GetValue() report value extraction (usbhid-ups) #1055
Conversation
Adds source and updates Makefile.am to build tests/getvaluetest, a test harness for the report value extraction function GetValue() in hidparser.c. getvaluetest has some built-in test cases, which are easily extensible, but also accepts a single test specification on the command line to allow for easy experimentation. getvaluetest -h for usage
|
@clepple, @jimklimov -- ping? The travis-ci seems hung up so this hasn't passed a required check yet, but it is actually ready to go. |
|
Hi @jimklimov -- is there something I need to do to help this PR along? I've updated it with respect to the latest master branch. The current shellcheck failures aren't in code I'm modifying. |
|
Sorry for the lag, I was fixing up a new CI before August and went for some
vacation afterwards, so did not have time to check this out. Probably in
September ...
…On Wed, Aug 18, 2021, 21:03 Nick Briggs ***@***.***> wrote:
Hi @jimklimov <https://github.com/jimklimov> -- is there something I need
to do to help this PR along? I've updated it with respect to the latest
master branch. The current shellcheck failures aren't in code I'm modifying.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1055 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMPTFCYWECLRKAHZMEIWALT5P7YRANCNFSM47TUCIAQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>
.
|
|
Thanks, no worries then. Vacation is always good. |
|
Thanks for staying tuned, now that CI is usable again, I'm looking at stalled PRs - sorry for that. Another one eventually looming would be the code-style to be consistent with other NUT codebase (4-space or TAB indentation, same style per file, would be one of those). Content-wise, the PR seems okay even so, but after a merge these files would likely get revised, manually or by some tooling :) I hope to get a SPARC (VM?) into the farm too, or at least some other arch (arm? mips?) to see if endianness issues pop up like that - or that none new appeared. What would be your guess, is a VM SPARC Linux (likely via qemu-static) or BSD sufficient for that, if I can't get some Solaris hardware to the farm? |
Include NUT "common.h" for NUT_UNUSED_VARIABLE among other things
|
No worries. Thanks for making the updates. I think that any big-endian system, emulated or real, would have found the problem that I ran into. I'm not sure which OSs are built for big-endian processors these days, but if there's a SPARC64 Linux that might be reasonable if a system running Solaris on SPARC isn't doable. Regarding the code-style: I see you have an "indent.sh" script -- it would be handy to have a .clang-format that encoded your recommended code style as it's a lot more powerful than basic indent, and probably could be added to the automation tooling. |
|
Yes, a clang-format integration was in the deep-drawer plan. There is actually an implementation in zeromq/zproject (recipe and code generator) that I contributed to and that cross-pollinated with NUT recipes, so adding the mechanism is not too hard. Coding the rules which fit our text and most of the source is the trouble :) |
|
Hmm, the test did run, but complained actually, see e.g. https://ci.networkupstools.org/blue/organizations/jenkins/nut%2Fnut/detail/PR-1055/4/pipeline/1447 and any orange balls in the build matrix above. At least on the first I clicked, it says: while others passed. |
|
The (failing) test result is correct, if you haven't yet merged my other PR that fixes the problem. |
|
Oh, right - thanks!
…On Thu, Sep 16, 2021, 23:32 Nick Briggs ***@***.***> wrote:
The (failing) test result is correct, if you haven't yet merged my other
PR that fixes the problem.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1055 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMPTFF37VYOFB5RQ2MBXU3UCJO5HANCNFSM47TUCIAQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
…type of sizeof-derived iterator
…23_GetValue_testharness See also #1
|
FYI: With some recent development on CI side, I made a branch that should combine QEMU testing and proposed LP64 fix and test from #1040 and #1055 ... "so here goes nothing" : https://ci.networkupstools.org/job/nut/job/nut/job/issue_1023_GetValue_qemu_test/ |
jimklimov
added
the
USB-HID encoding/LogMin/LogMax
Adds source and updates Makefile.am to build tests/getvaluetest,
a test harness for the report value extraction function GetValue()
in hidparser.c.
getvaluetest has some built-in test cases, which are easily extensible,
but also accepts a single test specification on the command line to
allow for easy experimentation.
getvaluetest -h for usage
This code is independent and could be merged before PR #1040 so that it's easy to check (on a 64-bit system) for the before/after behavior of
GetValue().