Added the script for issue #156 and edited the yml file to include it #1
Conversation
|
Description is empty. |
.travis.yml
Outdated
| coverage run --branch --source tlsfuzzer -m unittest discover; | ||
| fi | ||
| - coverage report -m | ||
| - python verify-scripts-json.py |
There was a problem hiding this comment.
I'd add a comment line documenting what is this supposed to do, so someone seeing a failure here can understand what he's supposed to fix
verify-scripts-json.py
Outdated
| missing = [] | ||
| missingrns = [] | ||
| tlslite = json.load(open(os.path.join(dir,'tests/tlslite-ng.json'))) | ||
| tlslitern = json.load(open(os.path.join(dir,'tests/tlslite-ng-random-subset.json'))) |
There was a problem hiding this comment.
do you need to check here? My understanding is that tlsline-ng.json would have everything, but I haven't really checked
There was a problem hiding this comment.
Indeed tlsline-ng.json is only used in travis but according to the issue description "Add script that runs in Travis that will verify that all scripts in scripts/are mentioned at least once in every json file.", so I am not really sure how is it intended to work exactly.
verify-scripts-json.py
Outdated
| missingrns.append(f) | ||
|
|
||
| if not missing and not missingrns: | ||
| print(" All scripts are int the json files") |
verify-scripts-json.py
Outdated
| print("There are " + str(len(missing)) + " scripts that are missing on tlslite-ng.json : ") | ||
| print("\n".join(missing)) | ||
| print("There are " + str(len(missingrns)) + " scripts that are missing on tlslite-ng.json : ".format(len(missingrns))) | ||
| print("\n".join(missingrns)) |
There was a problem hiding this comment.
shouldn't this script differentiate between success and failure based on error code? The convention is to return (0) on success and (1) on failure.
Do you have some way to test whether and how does this script work?
.travis.yml
Outdated
| coverage run --branch --source tlsfuzzer -m unittest discover; | ||
| fi | ||
| - coverage report -m | ||
| - python verify-scripts-json.py |
There was a problem hiding this comment.
Another improvement could be to simplify the script so that it is run twice, as:
python verify-scripts-json.py tests/tlslite-ng.json
python verify-scripts-json.py tests/tlslite-ng-random-subset.json
Although that seems like a waste it has the advantage that I, for example, can use it to test using another file in my home directory. That's actually a common case as it is expected for one to write such a json file for other implementations than tlslite.
There was a problem hiding this comment.
Yes it makes sense, fixed that
gekios
force-pushed
the
changes-for-issue156
branch
from
16fc628 to
52a8c19
Compare
Added the script for issue #156 and edited the yml file to include it
Description
I have added the script for issue #156 and included it in the yml file.
Motivation and Context
It solves issue #156
Checklist
tlslite-ng.jsonandtlslite-ng-random-subset.json