Skip to content

Conversation

@troglobit
Copy link
Contributor

Description

A mix of test framework fixes and improvements extracted from the firewall branch for separate review.

Checklist

Tick relevant boxes, this PR is-a or has-a:

  • Bugfix
    • Regression tests
    • ChangeLog updates (for next release)
  • Feature
    • YANG model change => revision updated?
    • Regression tests added?
    • ChangeLog updates (for next release)
    • Documentation added?
  • Test changes
    • Checked in changed Readme.adoc (make test-spec)
    • Added new test to group Readme.adoc and yaml file
  • Code style update (formatting, renaming)
  • Refactoring (please detail in commit messages)
  • Build related changes
  • Documentation content changes
    • ChangeLog updated (for major changes)
  • Other (please describe): infamy

@troglobit troglobit requested review from mattiaswal and wkz September 28, 2025 02:32
@troglobit troglobit added the ci:main Build default defconfig, not minimal label Sep 28, 2025
@troglobit troglobit enabled auto-merge September 28, 2025 02:32
@troglobit
Copy link
Contributor Author

We got a "cut off" job here due to duplicate build, but the Qemu test results are here https://github.com/kernelkit/infix/actions/runs/18068123992

Copy link
Contributor

@wkz wkz left a comment

Choose a reason for hiding this comment

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

We don't necessarily get rid of test.fail() now. It was just something that struck me as I read the log message.

A very common pattern in our tests is:

  if (condition):
    print(f"the condition failed with {output}")
    test.fail()

This change allows us to write:

  if (condition):
    test.fail(f"the condition failed with {output}")

Signed-off-by: Joachim Wiberg <[email protected]>
Before this change:

  ok 2 - Configure basic end-device firewall
  not ok 3 - Verify unused interface assigned to default zone
  # Exiting (2025-09-25 11:33:00)
  # Traceback (most recent call last):
  #   File "/home/jocke/src/x-misc/test/./case/infix_firewall/basic/test.py", line 127, in <module>
  #     assert unused_if not in public_zone["interface"], \
  #            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  # AssertionError: Unused interface e4 should be in default zone 'public', got interfaces: ['e2', 'e3', 'e5', 'e7', 'e8']

After this change:

  ok 2 - Configure basic end-device firewall
  not ok 3 - Verify unused interface assigned to default zone
  # Exiting (2025-09-25 11:35:00)
  #   File "/home/jocke/src/x-misc/test/./case/infix_firewall/basic/test.py", line 127, in <module>
  #     assert unused_if not in public_zone["interface"], \
  #            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  # Unused interface e4 should be in default zone 'public', got interfaces: ['e2', 'e3', 'e5', 'e7', 'e8']

Slightly shorter and arguably easier to read for a non-pythonic human.

Signed-off-by: Joachim Wiberg <[email protected]>
The xpath_to_uri() method only processed the first predicate in XPath
expressions with multiple [key='value'] patterns.  Each re.sub() call
was performed on the original xpath instead of the result the previous
substitutions, causing subsequent predicates to be ignored.

Example XPath that would fail:

    /infix-firewall:firewall/zone[name='untrusted']/interface[.='e2']

Would incorrectly convert to:

    /infix-firewall:firewall/zone[name='untrusted']/interface=e2

Instead of the correct RESTCONF URL:

    /infix-firewall:firewall/zone=untrusted/interface=e2

Signed-off-by: Joachim Wiberg <[email protected]>
@troglobit troglobit merged commit fe8610b into main Sep 29, 2025
6 checks passed
@troglobit troglobit deleted the infamy branch September 29, 2025 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:main Build default defconfig, not minimal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants