Skip to content

Tests fixes for clang#422

Merged
evelikov merged 3 commits intodkms-project:masterfrom
evelikov:tests-fixes-for-clang
Jun 6, 2024
Merged

Tests fixes for clang#422
evelikov merged 3 commits intodkms-project:masterfrom
evelikov:tests-fixes-for-clang

Conversation

@evelikov
Copy link
Collaborator

@evelikov evelikov commented Jun 6, 2024

Should address the issues raised in #417 (comment)

Namely:

  • modinfo output can vary - I suspect it prints the entries the way they're stored/parsed off the module
  • on (expected) failing tests, we don't print the CC/LD/OBJDUMP flags - just use a place holder

@ltsdw can you please test this PR on your end? I wrote it blindly, so I'm not 100% sure it addresses all the issues.

@evelikov evelikov force-pushed the tests-fixes-for-clang branch from 2b535c7 to d4c5bc2 Compare June 6, 2024 17:14
@ltsdw
Copy link
Contributor

ltsdw commented Jun 6, 2024

Actually, right now #417 sets OBJCOPY, not OBJDUMP. 😅

@evelikov
Copy link
Collaborator Author

evelikov commented Jun 6, 2024

Right - let me grab some coffee. Seems like are a few brown paper bag fixes needed 😊

@ltsdw
Copy link
Contributor

ltsdw commented Jun 6, 2024

Also at line 506 and 631 modinfo_quad isn't visible for the new spawned shell we would have to export the function first.

Also changed the mentions of OBJDUMP for OBJCOPY, all tests passing :)

Also changed the egrep for grep -E, the tests would fail otherwise +egrep: warning: egrep is obsolescent; using grep -E

dkms_tests_output.txt

--- a	2024-06-06 14:43:27.889088274 -0300
+++ run_test.sh	2024-06-06 14:40:03.572622287 -0300
@@ -62,7 +62,7 @@
 # Reportedly in some cases the entries in the modinfo output are ordered
 # differently. Fetch whatever we need and sort them.
 modinfo_quad() {
-    modinfo $1 | egrep "^description:|^filename:|^license:|^version:" | sort
+    modinfo $1 | grep -E "^description:|^filename:|^license:|^version:" | sort
 }
 
 SIGNING_MESSAGE=""
@@ -196,8 +196,8 @@
     sed -i "/^python3: can't open file '\/usr\/share\/apport\/package-hooks\/dkms_packages.py'\: \[Errno 2\] No such file or directory$/d" ${output_log}
     sed -i "/^ERROR (dkms apport): /d" ${output_log}
 
-    # Swap any CC/LD/OBJDUMP flags (if set) with a placeholder message
-    sed -i "s|\(make -j1 KERNELRELEASE=${KERNEL_VER} all\).*|\1 <omitting possibly set CC/LD/OBJDUMP flags>|" ${output_log}
+    # Swap any CC/LD/OBJCOPY flags (if set) with a placeholder message
+    sed -i "s|\(make -j1 KERNELRELEASE=${KERNEL_VER} all\).*|\1 <omitting possibly set CC/LD/OBJCOPY flags>|" ${output_log}
 }
 
 run_with_expected_output() {
@@ -503,7 +503,7 @@
 EOF
 
 echo 'Checking modinfo'
-run_with_expected_output sh -c "modinfo_quad /lib/modules/${KERNEL_VER}/${expected_dest_loc}/dkms_test.ko${mod_compression_ext}" << EOF
+run_with_expected_output sh -c "$(declare -f modinfo_quad); modinfo_quad /lib/modules/${KERNEL_VER}/${expected_dest_loc}/dkms_test.ko${mod_compression_ext}" << EOF
 description:    A Simple dkms test module
 filename:       /lib/modules/${KERNEL_VER}/${expected_dest_loc}/dkms_test.ko${mod_compression_ext}
 license:        GPL
@@ -628,7 +628,7 @@
 fi
 
 echo 'Checking modinfo'
-run_with_expected_output sh -c "modinfo_quad /lib/modules/${KERNEL_VER}/${expected_dest_loc}/dkms_test.ko${mod_compression_ext}" << EOF
+run_with_expected_output sh -c "$(declare -f modinfo_quad); modinfo_quad /lib/modules/${KERNEL_VER}/${expected_dest_loc}/dkms_test.ko${mod_compression_ext}" << EOF
 description:    A Simple dkms test module
 filename:       /lib/modules/${KERNEL_VER}/${expected_dest_loc}/dkms_test.ko${mod_compression_ext}
 license:        GPL
@@ -1428,7 +1428,7 @@
 Cleaning build area...
 Building module(s)...(bad exit status: 2)
 Failed command:
-make -j1 KERNELRELEASE=${KERNEL_VER} all <omitting possibly set CC/LD/OBJDUMP flags>
+make -j1 KERNELRELEASE=${KERNEL_VER} all <omitting possibly set CC/LD/OBJCOPY flags>
 Error! Bad return status for module build on kernel: ${KERNEL_VER} (${KERNEL_ARCH})
 Consult /var/lib/dkms/dkms_failing_test/1.0/build/make.log for more information.
 dkms autoinstall on ${KERNEL_VER}/${KERNEL_ARCH} failed for dkms_failing_test(10)
@@ -1447,7 +1447,7 @@
 Cleaning build area...
 Building module(s)...(bad exit status: 2)
 Failed command:
-make -j1 KERNELRELEASE=${KERNEL_VER} all <omitting possibly set CC/LD/OBJDUMP flags>
+make -j1 KERNELRELEASE=${KERNEL_VER} all <omitting possibly set CC/LD/OBJCOPY flags>
 Error! Bad return status for module build on kernel: ${KERNEL_VER} (${KERNEL_ARCH})
 Consult /var/lib/dkms/dkms_failing_test/1.0/build/make.log for more information.
 dkms autoinstall on ${KERNEL_VER}/${KERNEL_ARCH} failed for dkms_failing_test(10)
@@ -1596,7 +1596,7 @@
 Cleaning build area...
 Building module(s)...(bad exit status: 2)
 Failed command:
-make -j1 KERNELRELEASE=${KERNEL_VER} all <omitting possibly set CC/LD/OBJDUMP flags>
+make -j1 KERNELRELEASE=${KERNEL_VER} all <omitting possibly set CC/LD/OBJCOPY flags>
 Error! Bad return status for module build on kernel: ${KERNEL_VER} (${KERNEL_ARCH})
 Consult /var/lib/dkms/dkms_failing_test/1.0/build/make.log for more information.
 
@@ -1970,4 +1970,3 @@
 echo
 
 echo 'All tests successful :)'

@evelikov evelikov force-pushed the tests-fixes-for-clang branch from d4c5bc2 to f9fe97b Compare June 6, 2024 18:35
evelikov added 3 commits June 6, 2024 19:52
As the inline comment says, it has been reported that the ordering of
fields varies. That leads to the tests failing, so let's apply a quick
workaround.

I'm inclined to proposed a fix in kmod that produces deterministic
ordering, but that's for another day... Or if you're reading this,
please do beat me to it ;-)

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
The latter is the proper words for what we do (US spelling but meh).

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
The chances of failure when the flags are set is reasonably small
(famous last words), so omit them. This allows us to handle any
variation without adding heaps of extra code.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
@evelikov evelikov force-pushed the tests-fixes-for-clang branch from f9fe97b to b2ca691 Compare June 6, 2024 18:52
@evelikov
Copy link
Collaborator Author

evelikov commented Jun 6, 2024

Hmm why on earth do we do sh -C "foobar" again? Our run_with_... also seems a little broken, in that it passes the full command line including the expected error code to stdbuf. Overall the layering seems a bit ...

For now let's use your suggestions, thanks o/

@evelikov evelikov merged commit 31c5ce3 into dkms-project:master Jun 6, 2024
@evelikov evelikov deleted the tests-fixes-for-clang branch June 6, 2024 20:22
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.

2 participants