Skip to content
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

Update last tests for first version of test specification #684

Merged
merged 6 commits into from
Oct 4, 2024

Conversation

mattiaswal
Copy link
Contributor

@mattiaswal mattiaswal commented Oct 3, 2024

Description

Fix #606

Other information

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?
  • Code style update (formatting, renaming)
  • Refactoring (please detail in commit messages)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Copy link
Contributor

@troglobit troglobit left a comment

Choose a reason for hiding this comment

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

Nice work! I see now the container tests are really difficult to document, e.g., in comparison to DHCP client.

For the "test all possible interface combinations" test, @ahmkar94, me and wkz talked about how to describe networks that are internal to the DUT, like VETH pairs and bridges, that have no physical counterpart or connection to the host -- i.e., that we cannot describe in the topology.dot file. One option, that somehow was abandoned, was an ASCII art picture included in the top test docstring that would be translated to the Readme.adoc file. Maybe the container tests would benefit even more from this?

Example:

  .-------------.         .---------------.     .--------.
  |      |  tgt |---------| mgmt |        |     |  web-  |
  | host | data |---------| data | target |     | server |
  '-------------'         '---------------'     '--------'
                             |                    /
                            br0                  /
                              `----- veth0 -----'

test/case/infix_containers/container_bridge/test.py Outdated Show resolved Hide resolved
test/case/infix_containers/container_bridge/test.py Outdated Show resolved Hide resolved
test/case/infix_containers/container_veth/test.py Outdated Show resolved Hide resolved
@mattiaswal
Copy link
Contributor Author

Nice work! I see now the container tests are really difficult to document, e.g., in comparison to DHCP client.

For the "test all possible interface combinations" test, @ahmkar94, me and wkz talked about how to describe networks that are internal to the DUT, like VETH pairs and bridges, that have no physical counterpart or connection to the host -- i.e., that we cannot describe in the topology.dot file. One option, that somehow was abandoned, was an ASCII art picture included in the top test docstring that would be translated to the Readme.adoc file. Maybe the container tests would benefit even more from this?

Example:

  .-------------.         .---------------.     .--------.
  |      |  tgt |---------| mgmt |        |     |  web-  |
  | host | data |---------| data | target |     | server |
  '-------------'         '---------------'     '--------'
                             |                    /
                            br0                  /
                              `----- veth0 -----'

Yes, it has been a struggle, An extra ASCII art I think will do it more understandable. But for now this will do, but of course you are right. We need do describe software-ports in a better way.

@mattiaswal mattiaswal merged commit a66d1d3 into main Oct 4, 2024
6 checks passed
@mattiaswal mattiaswal deleted the fix-test-texts branch October 4, 2024 11:12
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.

Update all tests for test specification
2 participants