Add canonical OC to test README template #3336
Merged
Conversation
This file contains 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
Pull Request Test Coverage Report for Build 11804359535Details
💛 - Coveralls |
robshakir
reviewed
Jul 30, 2024
|
Whether it's JSON or YAML, as someone trying to produce openconfig for a device I would like to have a concrete stanza to model my configuration on. |
sachendras
added a commit
that referenced
this pull request
Aug 30, 2024
Changing the config to the approach suggested in #3336
sachendras
added a commit
that referenced
this pull request
Sep 4, 2024
* Update to the MTU test readme to cover for Tunnel interfaces. - Separate the test to 2 sub tests - While the original test covered MTU configuration on physical/Bundle interfaces only, added subtest covers for Tunnel interface MTU configuration and corresponding emphasis on zero fragmentation plus packet drops. Updated write-up also covers for missed config and state paths. * Update README.md Added additional clarity on the tunnel interface MTU config that it isn't mandatory for the NOS to allow for MTU config on tunnel interfaces. Expectation is for the tunnel interface to respect the MTU configured on the underlying physical or bundle interface acting as an egress point. * Update README.md updated the following bullet for MTU-1.3.2 Ensure MTU configured on this tunnel interface is in context to the MTU on the egress physical/bundle interface. It is acceptable if the implementation doesn't support explicit MTU config on tunnel interfaces. Idea is for the tunnel interface to respect the egress physical/bundle interface MTU config. * Update README.md * Update README.md Corrected Yaml error * Update README.md * Update README.md * Update README.md Updated paths that were incorrectly defined. * Update README.md Corrected the OC path /interfaces/interface/tunnel/config/dst: It was originally /interfaces/interface/tunnel/config/dest: * Adding canonical OC to config path Changing the config to the approach suggested in #3336 * Update README.md Trying to correct the error Run go install ./tools/validate_readme_spec ########## READMEs in changed directories to be validated (including ones to be exempted): feature/mtu/largeippacket/otg_tests/large_ip_packet_transmission/README.md ########## Validating READMEs in changed directories: I0830 17:54:13.842665 4350 validate_readme_spec.go:149] Validating "feature/mtu/largeippacket/otg_tests/large_ip_packet_transmission/README.md" E0830 17:54:13.843301 4350 validate_readme_spec.go:156] file feature/mtu/largeippacket/otg_tests/large_ip_packet_transmission/README.md: mdocspec: error parsing YAML: yaml: unmarshal errors: line 4: cannot unmarshal !!seq into map[string]interface {} F0830 17:54:13.843594 4350 validate_readme_spec.go:178] The following files have errors: feature/mtu/largeippacket/otg_tests/large_ip_packet_transmission/README.md Error: Process completed with exit code 1. * Update README.md * Update README.md Moved the canonical config to the Test procedure area. Also, restored the section under ## OpenConfig Path and RPC Coverage to how it was before.
frasieroh
pushed a commit
to aristanetworks/openconfig-featureprofiles
that referenced
this pull request
Sep 5, 2024
…nfig#3401) * Update to the MTU test readme to cover for Tunnel interfaces. - Separate the test to 2 sub tests - While the original test covered MTU configuration on physical/Bundle interfaces only, added subtest covers for Tunnel interface MTU configuration and corresponding emphasis on zero fragmentation plus packet drops. Updated write-up also covers for missed config and state paths. * Update README.md Added additional clarity on the tunnel interface MTU config that it isn't mandatory for the NOS to allow for MTU config on tunnel interfaces. Expectation is for the tunnel interface to respect the MTU configured on the underlying physical or bundle interface acting as an egress point. * Update README.md updated the following bullet for MTU-1.3.2 Ensure MTU configured on this tunnel interface is in context to the MTU on the egress physical/bundle interface. It is acceptable if the implementation doesn't support explicit MTU config on tunnel interfaces. Idea is for the tunnel interface to respect the egress physical/bundle interface MTU config. * Update README.md * Update README.md Corrected Yaml error * Update README.md * Update README.md * Update README.md Updated paths that were incorrectly defined. * Update README.md Corrected the OC path /interfaces/interface/tunnel/config/dst: It was originally /interfaces/interface/tunnel/config/dest: * Adding canonical OC to config path Changing the config to the approach suggested in openconfig#3336 * Update README.md Trying to correct the error Run go install ./tools/validate_readme_spec ########## READMEs in changed directories to be validated (including ones to be exempted): feature/mtu/largeippacket/otg_tests/large_ip_packet_transmission/README.md ########## Validating READMEs in changed directories: I0830 17:54:13.842665 4350 validate_readme_spec.go:149] Validating "feature/mtu/largeippacket/otg_tests/large_ip_packet_transmission/README.md" E0830 17:54:13.843301 4350 validate_readme_spec.go:156] file feature/mtu/largeippacket/otg_tests/large_ip_packet_transmission/README.md: mdocspec: error parsing YAML: yaml: unmarshal errors: line 4: cannot unmarshal !!seq into map[string]interface {} F0830 17:54:13.843594 4350 validate_readme_spec.go:178] The following files have errors: feature/mtu/largeippacket/otg_tests/large_ip_packet_transmission/README.md Error: Process completed with exit code 1. * Update README.md * Update README.md Moved the canonical config to the Test procedure area. Also, restored the section under ## OpenConfig Path and RPC Coverage to how it was before.
xw-g
previously requested changes
Sep 30, 2024
|
@xw-g @robshakir ready for re-review. @self-maurya @rohit-rp, please feel free to provide any suggestions. |
self-maurya
previously approved these changes
Oct 17, 2024
robshakir
reviewed
Oct 22, 2024
robshakir
reviewed
Oct 22, 2024
robshakir
reviewed
Oct 22, 2024
Co-authored-by: Rob Shakir <[email protected]>
robshakir
approved these changes
Nov 12, 2024
dplore
dismissed
xw-g’s stale review
Addressed comment, removed yaml and added json
mastarkey
pushed a commit
to b4firex/featureprofiles
that referenced
this pull request
Nov 18, 2024
* Update test-requirements-template.md to require Canonical OC section, written in JSON format
ANISH-GOTTAPU
pushed a commit
to open-traffic-generator/featureprofiles
that referenced
this pull request
Nov 28, 2024
* Update test-requirements-template.md to require Canonical OC section, written in JSON format
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.
Thoughts on this?
The goal is to have an easy to read, reference/golden/canonical configuration that covers the test topic. It should be a snippet including the paths that are to be tested.
The format could be JSON or YAML, but I think YAML is considerably easier to read and somewhat easier to write.
The YAML could be validated by converting to JSON and then parsed by ygot or gnmic.