Skip to content

Conversation

@tomplus
Copy link
Contributor

@tomplus tomplus commented Nov 15, 2017

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Fixes related to:

  1. Travis doesn't know that tests fail because an exit codes from bash scripts are always zero - fixed
  2. Tornado client doesn't work in python < 2.7.9 - fixed
  3. Some tests are skipped because public PetStore endpoint has incorrect data. Travis use local instance of PetStore server and all test can run. I removed @skip and also force endpoint name to localhost. IMO it'll remind developers about starting a local instance. Test endpoint returns invalid data #6907

Please take a look: @wing328 @taxpon @frol @mbohlool @cbornet @kenjones-cisco

@frol
Copy link
Contributor

frol commented Nov 15, 2017

Travis doesn't know that tests fail because an exit codes from bash scripts are always zero - fixed

Does this mean that flake8 failure will be also ignored? Should we use set -e or run the scripts with explicit bash -e ./test_python2_and_3.sh?

@tomplus
Copy link
Contributor Author

tomplus commented Nov 15, 2017

Travis sees an exit code from a script but it's exit code from last command. So we have situation where python test failed but flake8 "covers" it and returns 0. Now I exit immediately if python tests failed. flake8 is a last command so its code will be interpreted correctly. If flake return non-zero code, Travis will notice it.

I consider using set -e but I think it would be too big change in this quite simple case.

@tomplus
Copy link
Contributor Author

tomplus commented Nov 15, 2017

Tornado client doesn't work in python < 2.7.9 - fixed

Finally I had to drop compatibility with older Python. It's more complicated and I'll fix it in next PR.

@wing328 wing328 added this to the v2.3.0 milestone Nov 16, 2017
@frol
Copy link
Contributor

frol commented Nov 16, 2017

Looks good to me!

@wing328
Copy link
Contributor

wing328 commented Nov 16, 2017

cc @toumorokoshi as well

@wing328 wing328 merged commit 73cb68e into swagger-api:master Nov 17, 2017
@tomplus tomplus mentioned this pull request Nov 26, 2017
4 tasks
@tomplus tomplus deleted the fix/python-test branch December 20, 2017 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants