Repository navigation
Add a bunch of unit tests - #305
Merged
Merged
Conversation
josuedhg
reviewed
Sep 2, 2026
Member
Author
I don't disagree, but it's huge amounts of boiler plate and I don't really have a better way - parameterizing everything and moving the decls out doesn't make it any cleaner. I welcome suggestions to make it better. |
I have not an answer, I was thinking exactly in parameterize everything to move the declarations out as you mention but you are right the amount of parameters would be needed would be also not good at all, then I have not answer or suggestion on how to do it better I will just not complain about it. |
vathpela
force-pushed
the
unit-tests
branch
2 times, most recently
from
September 11, 2026 16:33
1acb7e9 to
3fe917e
Compare
This adds a framework for unit tests, and some tests for some of the basic device path making and formatting functions. Signed-off-by: Peter Jones <pjones@redhat.com>
In the previous patch, we added $(quiet) and $(rmverbose) as options to be enabled with V=1. This patch makes our makefiles use them extensively. Signed-off-by: Peter Jones <pjones@redhat.com>
Currently efidp_data_address() does its whole job wrong, because it gives you the address after the entire efidp union instead of after the header, and that can never be correct. Anything using it therefore clobbers random memory. This makes it use the header size to find the position. Signed-off-by: Peter Jones <pjones@redhat.com>
A reviewer noted that some of our tests aren't invariant because they never touch reserved space, and nothing ever zeroes that space when reserving it. This change makes efidp_make_generic() zero the space up to the size of the object being initialized, but not the following yet-unallocated buffer. It also clarifies the definition of the function, and fixes the test case to properly pass the right buffers and test the right size outcomes. Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Currently we're specifying "-fprofile-arcs -ftest-coverage --coverage", on gcc, and similar on clang. The latter is an alias that includes the former two. This patch just uses the last "--coverage". Signed-off-by: Peter Jones <pjones@redhat.com>
_format_acpi_dn() never got support for handling parallel ports correctly. Whoops. Signed-off-by: Peter Jones <pjones@redhat.com>
In _format_acpi_dn(), we were using the wrong format specifier for HID devices without symbolic names, making it hard to test correctly. This uses PRIx32 correctly. Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
efidp_make_emmc() accidentally uses the device type EFIDP_MSG_NVME, which of course makes any attempt to use it show up somewhat wrong. This fixes it to use EFIDP_MSG_EMMC. Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
When formatting a UCS-2 string in a device path, we allocate a buffer on the stack to write the utf8 version of it in to. Unfortunately this doesn't correctly account for the size utf8 multi-byte characters. This fixes that check. Resolves: RHEL-216105 Signed-off-by: Peter Jones <pjones@redhat.com>
_format_media_dn() gets the offset of a usb wwid serial number rather than a file when allocating space for a file. This is wrong. This changes it to use the file offset instead. Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
While writing a test case, I discovered two issues with efidp_set_node_data(): it could never work, and sometimes it's wrong about why. If you specify a buffer that's too large, efidp_set_node_data() currently gives you the error message that the device node is smaller than a correctly formed header. This isn't correct. This is triggered by a bug in efidp_data_address(), wherein it computes data offset with the const_efidp size, which is a union of the possible types, rather than the header size. This is very wrong! This changes it to actually explain what went wrong in the first case. The efidp_data_address() bug will be fixed separately. Signed-off-by: Peter Jones <pjones@redhat.com>
Currently if you get to the end of a device path and call efidp_get_next_end(node, &node) you'll just get the same node no matter what. In some sense this makes sense with END_ENTIRE, since we can't guarantee it's safe to read the next memory location, but this makes no sense at all if it's an END_INSTANCE, which is really when you want to be doing this anyway. This makes it keep going on END_INSTANCE. Signed-off-by: Peter Jones <pjones@redhat.com>
Currently if a DP has an END_INSTANCE node in the middle (where you'd expect it), we accidentally put two commas instead of one, and then we erroneously start the next DP instance with the node separator mark (/), i.e. you'll see: PciRoot(0x1),,/PciRoot(0x2) Where you should see: PciRoot(0x1),PciRoot(0x2) This fixes those to just be a comma. Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
This attempts to directly test the device path utility functions, such as: efidp_set_node_data() efidp_size() efidp_node_size() efidp_type() efidp_subtype() Signed-off-by: Peter Jones <pjones@redhat.com>
efidp_make_ipv6() just never appears to have been implemented. Here goes! Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
This also changes the implementation quite a bit because the test showed some ways it was lacking. Signed-off-by: Peter Jones <pjones@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds a unit test framework as well as a bunch of unit tests, primarily to the device path parsing and formatting code, while fixing a bunch of bugs along the way. It's not exhaustive, but it's a big step forward.