Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tests reporting (upstream) #794

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jimklimov
Copy link

@jimklimov jimklimov commented Jan 26, 2025

Another part of work, done in NUT fork of libmodbus as a subset of networkupstools#3, ripe for up-streaming to minimize context differences for eventual RTU USB contribution as such.

This PR focuses on usability of the libmodbus test suite to investigate problems seen by make check in diverse platform builds (including FreeBSD, OpenBSD, a couple of illumos/Solaris distros, MacOS+HomeBrew, Linux, Linux+MinGW and MSYS2 for Windows target).

This selection of commits does not include any major attempts to fix the issues seen on those workers, but rather a large mostly cosmetic change set to represent the test progress:

  • in tests/unit-test-client.c, a TEST_TITLE() macro was introduced to report the beginning of a test case, so that subsequent error messages issued by the library methods called by unit-test-client can be attributed to this test case and aid in troubleshooting. The title string is remembered as last_test_title[] and reported in subsequent success or failure of ASSERT_TRUE() message(s). Program text was reshuffled to begin the test cases with the title, and adapt the wording where needed, but there should not be any functional changes in the test cases just yet.
  • as a separate commit spanning tests/unit-test.h.in and tests/unit-test-client.c, a FLUSHOUT() macro was introduced to facilitate debugging in Windows (GitBASH) console which tends to fluctuate between flushing out every character (so messing up reports from test server and client writing to same console) to buffering and only flushing every 4KB or so.
    • Anyhow, piping the outputs of client and server to dedicated cat's seems to have helped the situation a lot more than peppering the code with flushing or even configuring the behavior with setvbuf, as explored in the fork's original PR.
  • in tests/unit-test-server.c, added clearer logging about reasons of test set-up failure, if it happens.
  • finally, in tests/unit-test-server.c and tests/unit-test-client.c (with ip_or_device) and in tests/unit-test.h.in (with UT_BITS_ADDRESS_VAL), a few C fixes landed to help with some compilers and their view on standards and static analysis.

FYI: PR #792 seems like a useful addition to the test suite as well, although that case did not pop out in my experiments.

NOTE: CLA form posted a few weeks ago, ignored so far.

Copy link

cla-bot bot commented Jan 26, 2025

We require contributors to sign our Contributor License Agreement. In order for us to review and merge your code, please fill https://forms.gle/5635zjphDo5JEJQSA to get added. Your document will be manually checked by the maintainer. Be patient...

jimklimov and others added 6 commits January 26, 2025 15:24
Signed-off-by: Jim Klimov <jimklimov@gmail.com>
Signed-off-by: Jim Klimov <jimklimov@gmail.com>
…n test log; move the message to before the tested library method

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
… help with console logs on WIN32 test runs

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…_device is safe to use

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…ALID_REQUEST_LENGTH is initialized correctly with constexpr

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
@jimklimov jimklimov force-pushed the fix-mingw-tests-reporting-upstream branch from dae2dc6 to 670d4e0 Compare January 26, 2025 14:24
Copy link

cla-bot bot commented Jan 26, 2025

We require contributors to sign our Contributor License Agreement. In order for us to review and merge your code, please fill https://forms.gle/5635zjphDo5JEJQSA to get added. Your document will be manually checked by the maintainer. Be patient...

…est_title[]

Signed-off-by: Jim Klimov <jimklimov@gmail.com>
Copy link

cla-bot bot commented Jan 26, 2025

We require contributors to sign our Contributor License Agreement. In order for us to review and merge your code, please fill https://forms.gle/5635zjphDo5JEJQSA to get added. Your document will be manually checked by the maintainer. Be patient...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant