Skip to content

Create test harness for GetValue() report value extraction (usbhid-ups)#1055

Merged
jimklimov merged 15 commits intonetworkupstools:masterfrom
nbriggs:issue_1023_GetValue_testharness
Oct 15, 2021
Merged

Create test harness for GetValue() report value extraction (usbhid-ups)#1055
jimklimov merged 15 commits intonetworkupstools:masterfrom
nbriggs:issue_1023_GetValue_testharness

Conversation

@nbriggs
Copy link
Copy Markdown
Contributor

@nbriggs nbriggs commented Jul 1, 2021

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().

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
@nbriggs
Copy link
Copy Markdown
Contributor Author

nbriggs commented Jul 15, 2021

@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.

@nbriggs
Copy link
Copy Markdown
Contributor Author

nbriggs commented Aug 18, 2021

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.

@jimklimov
Copy link
Copy Markdown
Member

jimklimov commented Aug 19, 2021 via email

@nbriggs
Copy link
Copy Markdown
Contributor Author

nbriggs commented Aug 19, 2021

Thanks, no worries then. Vacation is always good.

@jimklimov
Copy link
Copy Markdown
Member

Thanks for staying tuned, now that CI is usable again, I'm looking at stalled PRs - sorry for that.
There was a small issue that CI complained of, fixed in-place.

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?

jimklimov and others added 2 commits September 15, 2021 11:58
Include NUT "common.h" for NUT_UNUSED_VARIABLE among other things
@nbriggs
Copy link
Copy Markdown
Contributor Author

nbriggs commented Sep 15, 2021

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.

@jimklimov
Copy link
Copy Markdown
Member

jimklimov commented Sep 16, 2021

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 :)

@jimklimov
Copy link
Copy Markdown
Member

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:

[2021-09-15T22:38:19.249Z] Test #1 buf "00 ff ff ff ff" offset 0 size 32 logmin -1 (0xffffffffffffffff) logmax 2147483647 (0x7fffffff) value 0 FAIL expected -1

while others passed.

@nbriggs
Copy link
Copy Markdown
Contributor Author

nbriggs commented Sep 16, 2021

The (failing) test result is correct, if you haven't yet merged my other PR that fixes the problem.

@jimklimov
Copy link
Copy Markdown
Member

jimklimov commented Sep 19, 2021 via email

@jimklimov
Copy link
Copy Markdown
Member

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/

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

Labels

USB-HID encoding/LogMin/LogMax Issues and solutions (PRs) specifically about incorrect values in bitstream

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants