Skip to content

(kernelci/api/helper) Fix sanitizer and add test #2876

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nuclearcat
Copy link
Member

@nuclearcat nuclearcat commented May 20, 2025

Since we have repeated mistake, we should add basic test case. Tests still need fixing, as some other of them are failing.

Fixes: kernelci/kernelci-pipeline#1157

@nuclearcat nuclearcat force-pushed the fix-sanitizer branch 5 times, most recently from 3786aca to 7121461 Compare May 20, 2025 08:10
Since we have repeated mistake, we should add basic test case.
Tests still need fixing, as some other of them are failing.

Signed-off-by: Denys Fedoryshchenko <[email protected]>
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds recursive sanitization of curly braces in node fields and a corresponding basic test to verify the behavior.

  • Update _fsanitize_node_fields to walk through nested dicts and lists when escaping { and }
  • Add tests/api/test_apihelper.py to validate that commit messages have their braces doubled

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/api/test_apihelper.py New test to assert that _fsanitize_node_fields correctly escapes curly braces in nested nodes
kernelci/api/helper.py Revamp of _fsanitize_node_fields to perform deep traversal on dicts and lists
Comments suppressed due to low confidence (4)

tests/api/test_apihelper.py:14

  • [nitpick] The test function name test_apihelper is too generic. Consider renaming it to test_fsanitize_node_fields_curly_braces to clarify what behavior is being validated.
def test_apihelper():

kernelci/api/helper.py:393

  • [nitpick] Docstring indentation is inconsistent and the second sentence is indented farther than the summary line. Align all docstring lines at the same indentation level and include a brief :returns: description.
Sanitize node fields to escape curly braces

tests/api/test_apihelper.py:11

  • The test calls kernelci.config.load(...) but never imports kernelci.config. Add import kernelci.config (or from kernelci import config) at the top of the file so kernelci.config is defined.
import kernelci.api

kernelci/api/helper.py:401

  • The recursive call self._fsanitize_node_fields(val, field_name) is invoked on every non-matching value, including primitives like strings and numbers. Restrict recursion to only dicts and lists to avoid needless calls on other types.
else:

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.

Fix crash in k8s scheduler
1 participant