feat: Add custom_allowlist to verify_messages function#7
Open
KTokarze wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for providing a custom allowlist when verifying dmesg messages, and extends unit tests to cover custom allowlist behavior.
Changes:
- Extend
verify_messages()to accept acustom_allowlistparameter instead of always usingDMESG_WHITELIST. - Add unit tests validating full-match, partial-match, and empty allowlist behaviors.
- Minor formatting fix in
constants.py.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
mfd_dmesg/base.py |
Adds custom_allowlist parameter and uses it during benign-message filtering. |
tests/unit/test_mfd_dmesg/test_base.py |
Adds new test cases for custom_allowlist scenarios (success/failure). |
mfd_dmesg/constants.py |
Fixes import indentation/formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return True | ||
|
|
||
| def verify_messages(self) -> dict: | ||
| def verify_messages(self, custom_allowlist: Optional[Iterable[str]] = None) -> dict: |
Comment on lines
+230
to
+231
| if custom_allowlist is None: | ||
| custom_allowlist = DMESG_WHITELIST |
Comment on lines
240
to
241
| for benign_message in custom_allowlist: | ||
| if benign_message in error: |
| return True | ||
|
|
||
| def verify_messages(self) -> dict: | ||
| def verify_messages(self, custom_allowlist: Optional[Iterable[str]] = None) -> dict: |
Comment on lines
240
to
241
| for benign_message in custom_allowlist: | ||
| if benign_message in error: |
| dmesg._connection.execute_command.return_value = ConnectionCompletedProcess( | ||
| return_code=0, args="command", stdout=output, stderr="stderr" | ||
| ) | ||
| result = dmesg.verify_messages(custom_allowlist=custom_allowlist) |
| dmesg._connection.execute_command.return_value = ConnectionCompletedProcess( | ||
| return_code=0, args="command", stdout=output, stderr="stderr" | ||
| ) | ||
| result = dmesg.verify_messages(custom_allowlist=custom_allowlist) |
adrianlasota
requested changes
May 25, 2026
| return True | ||
|
|
||
| def verify_messages(self) -> dict: | ||
| def verify_messages(self, custom_allowlist: Optional[Iterable[str]] = None) -> dict: |
Contributor
There was a problem hiding this comment.
use new typehints
Iterable[str] | None
mchromin
requested changes
May 25, 2026
39f0dde to
1242894
Compare
Comment on lines
224
to
+233
| """Verify if there are err level messages in dmesg output. | ||
|
|
||
| :param custom_allowlist: list of error messages to ignore in the dmesg output, if any. | ||
| By default, it uses DMESG_WHITELIST which contains known benign errors. | ||
| If this parameter is provided, it will be combined with DMESG_WHITELIST | ||
| to create the final allowlist. | ||
| :return: dictionary indicating success or failure and the error messages if present. | ||
| """ | ||
| if custom_allowlist is None: | ||
| allowlist = DMESG_WHITELIST |
Comment on lines
+389
to
+402
| def test_verify_messages_ignores_empty_lines(self, dmesg): | ||
| output = ( | ||
| "[ 4.923735] SELinux: Runtime disable is not supported\n" | ||
| "\n" | ||
| "[ 321.327775] CIFS: VFS: unaligned rsize, making it a multiple of 4096 bytes" | ||
| ) | ||
| custom_allowlist = ["SELinux", "unaligned rsize"] | ||
| dmesg._connection.execute_command.return_value = ConnectionCompletedProcess( | ||
| return_code=0, args="command", stdout=output, stderr="stderr" | ||
| ) | ||
| result = dmesg.verify_messages(custom_allowlist=custom_allowlist) | ||
| assert result["successful"] | ||
| assert result["error"] == "" | ||
|
|
Signed-off-by: Kacper Tokarzewski <kacper.tokarzewski@intel.com>
mchromin
approved these changes
May 28, 2026
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 pull request enhances the flexibility of the
verify_messagesmethod in themfd_dmesgmodule by allowing users to provide a custom allowlist of benign error messages to ignore. It also adds comprehensive unit tests to verify this new functionality and ensure correct behavior in various scenarios.Enhancements to allowlist functionality
verify_messagesmethod inbase.pynow accepts acustom_allowlistparameter, enabling callers to specify which error messages should be ignored during verification, instead of always using the defaultDMESG_WHITELIST.verify_messagesis updated to use the providedcustom_allowlistrather than the static whitelist, making the error checking more customizable.Testing improvements
test_base.pyto cover cases with a custom allowlist, partial matches, and an empty allowlist, ensuring the new functionality works as intended and edge cases are handled. [1] [2]Type hinting and imports
base.pyare updated to support the new parameter and maintain type safety, specifically by addingAnyto the imports.