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.

Sign up for GitHub

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

Jump to bottom

test: Update topologies and test texts #710

Merged
merged 1 commit into from
Oct 16, 2024
Merged

test: Update topologies and test texts #710

merged 1 commit into from
Oct 16, 2024

Conversation

mattiaswal
Copy link
Contributor

All mgmt links should be lightgrey, all mgmt ports should be named mgmt links between duts should be named link.

Description

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):

@troglobit
Copy link
Contributor

Wow, that was quick! Nice work 💯

@mattiaswal mattiaswal requested a review from axkar October 14, 2024 16:54
@mattiaswal mattiaswal force-pushed the fix-test-texts branch 3 times, most recently from 80b2512 to 1f8f642 Compare October 15, 2024 07:17
axkar
axkar requested changes Oct 15, 2024
View reviewed changes
Copy link
Collaborator

@axkar axkar left a comment

Choose a reason for hiding this comment

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

Great work! A few comments only :)

test/case/ietf_interfaces/bridge_vlan/test.py Show resolved Hide resolved
test/case/ietf_interfaces/bridge_vlan/topology.dot Outdated Show resolved Hide resolved
test/case/ietf_interfaces/bridge_vlan_separation/test.py Outdated Show resolved Hide resolved
test/case/ietf_interfaces/bridge_vlan_separation/topology.dot Outdated Show resolved Hide resolved
jovatn
jovatn approved these changes Oct 15, 2024
View reviewed changes
Copy link
Contributor

@jovatn jovatn left a comment

Choose a reason for hiding this comment

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

Great work!
See inline comments for proposed fixes - use what you think is useful.

A related note: OSPF multi-area still does not explain 11.0-network in topology or description, but mentioned in one of the test steps. Perhaps a small note in description, saying there are additional networks?

test/case/ietf_interfaces/bridge_vlan/topology.dot Outdated Show resolved Hide resolved
test/case/ietf_interfaces/dual_bridge/Readme.adoc Outdated Show resolved Hide resolved
test/case/ietf_interfaces/igmp_basic/Readme.adoc Outdated Show resolved Hide resolved
test/case/ietf_interfaces/dual_bridge/test.py Outdated Show resolved Hide resolved
test/case/ietf_interfaces/igmp_basic/test.py Outdated Show resolved Hide resolved
test/case/ietf_interfaces/igmp_basic/test.py Outdated Show resolved Hide resolved
test/case/ietf_interfaces/routing_basic/topology.dot Show resolved Hide resolved
test/case/ietf_interfaces/static_multicast_filters/test.py Show resolved Hide resolved
test/case/ietf_interfaces/static_multicast_filters/test.py Outdated Show resolved Hide resolved
test/case/ietf_interfaces/static_multicast_filters/test.py Outdated Show resolved Hide resolved
@mattiaswal mattiaswal requested a review from axkar October 15, 2024 11:58
@axkar axkar changed the title test: Update topogies and test texts test: Update topologies and test texts Oct 15, 2024
axkar
axkar approved these changes Oct 15, 2024
View reviewed changes
Copy link
Collaborator

@axkar axkar left a comment

Choose a reason for hiding this comment

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

🥇

@mattiaswal
test: Update topologies and test texts
34cb229 
All mgmt links should be lightgrey, all mgmt ports should be named mgmt
links between duts should be named link.
@mattiaswal
Copy link
Contributor Author

Great work! See inline comments for proposed fixes - use what you think is useful.

A related note: OSPF multi-area still does not explain 11.0-network in topology or description, but mentioned in one of the test steps. Perhaps a small note in description, saying there are additional networks?

Forgot that! There is now an ASCII-art over loopback configuration.

@mattiaswal mattiaswal merged commit 90bda1b into main Oct 16, 2024
6 checks passed
@mattiaswal mattiaswal deleted the fix-test-texts branch October 16, 2024 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@axkar axkar axkar approved these changes

@jovatn jovatn jovatn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mattiaswal @troglobit @jovatn @axkar