You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by "ocket8888 (via GitHub)" <gi...@apache.org> on 2023/03/23 06:01:29 UTC

[GitHub] [trafficcontrol] ocket8888 opened a new pull request, #7419: Various improvements to the API contract tests

ocket8888 opened a new pull request, #7419:
URL: https://github.com/apache/trafficcontrol/pull/7419

   This PR makes a bunch of changes to the newly added API contract tests. The changes fit into three categories:
   
   - typing errors - typing errors were reduced from 13 to 0
   - linting errors - linting score was improved from 1.86/10 to 10/10 with respect to the linting configuration file at `traffic_control/clients/python/pylint.rc`
   - configurability - I added the ability to specify locations for prerequisite files and configuration files using command-line option-arguments. I also added the use of the environment variables `TO_USER`, `TO_PASSWORD`, and `TO_URL`. Also, the API version and optional non-standard port are now parsed from the URL instead of double-specified.
   
   Those were the only things I wanted to accomplish with this PR, but I did run into two bugs that I fixed
   
   - if you use `--to-url`, it will use that URL to connect to Traffic Ops, but it'll use the API version parsed from whatever was in the config file.
   - API version is stored in a `float`; so version 4.10 will be indistinguishable from version 4.1 to the testing script
   
   
   <hr/>
   
   ## Which Traffic Control components are affected by this PR?
   <!-- Please delete all components from this list that are NOT affected by this PR.
   Feel free to add the name of a tool or script that is affected but not on the list.
   -->
   - Documentation
   - Traffic Control Cache Config (`t3c`, formerly ORT)
   - Traffic Control Health Client (tc-health-client)
   - Traffic Control Client <!-- Please specify which (Python, Go, or Java) -->
   - Traffic Monitor
   - Traffic Ops
   - Traffic Portal
   - Traffic Router
   - Traffic Stats
   - Grove
   - CDN in a Box
   - Automation <!-- Please specify which (GitHub Actions, Docker images, Ansible Roles, etc.) -->
   - unknown
   
   ## What is the best way to verify this PR?
   To verify no typing issues, use a static type checker. I suggest [Pyright](https://pypi.org/project/pyright/)  (I checked using `pyright` version 1.1.300). It should report 0 errors and 0 warnings.
   
   To verify no linting issues, use Pylint with our Pylint configuration file located at `traffic_ops/clients/python/pylint.rc` e.g.
   `pylint --rcfile=../../../../traffic_control/clients/python/pylint.rc ./conftest.py ./test_cdns.py`. It should report a perfect 10/10 score (I checked using Pylint 2.16.2 (asteroid 2.14.2)).
   
   To verify configurability, move/copy/rename the prerequisites file and the configuration file and then pass them into pytest using the `--prerequisites` and `--config` long options, respectively. The tests should pass as normal. You should also be able to get them to run and pass as normal by removing the configuration file entirely and just using the aforementioned environment variables (as is customary with some of our other tools e.g. `toget`).
   
   I also added a few docstring testable examples, but getting those to run is surprisingly annoying, so I wouldn't bother.
   
   ## PR submission checklist
   - [x] This PR has tests
   - [x] This PR has documentation
   - [x] This PR has a CHANGELOG.md entry
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] shamrickus merged pull request #7419: Various improvements to the API contract tests

Posted by "shamrickus (via GitHub)" <gi...@apache.org>.
shamrickus merged PR #7419:
URL: https://github.com/apache/trafficcontrol/pull/7419


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org