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.

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

clenaup openapi_parser_parse_alias test #159

Merged
merged 2 commits into from
Jun 20, 2020

Conversation

FlorianLudwig
Copy link
Contributor

cleaning up the test for better readability, especially on test failure.

@codecov
Copy link

codecov bot commented Jun 20, 2020

Codecov Report

Merging #159 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #159   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           12        12           
  Lines          916       916           
  Branches       187       187           
=========================================
  Hits           916       916           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2cc4d0d...28604fe. Read the comment docs.

@koxudaxi koxudaxi merged commit ef6f682 into koxudaxi:master Jun 20, 2020
@koxudaxi
Copy link
Owner

@FlorianLudwig

Thank you very much.

We may have to refactor all tests for better readability.
It may be an answer to write expected data in files.

How do you think?
example:
https://github.com/koxudaxi/fastapi-code-generator/blob/983c0bcb8978969a591bab242fbb6bcfa3c8e039/tests/test_generate.py

@FlorianLudwig
Copy link
Contributor Author

FlorianLudwig commented Jun 20, 2020

@koxudaxi
I like the idea of write expected data in files!

It might be a good chance to cleanup the tests overall - there seem to be quite a few tests testing the same thing.

One obvious example would be the test_main and test_main_kr. I think the test_main can be deleted as all tests are in the test_main_kr as well.

@koxudaxi koxudaxi mentioned this pull request Jun 20, 2020
@koxudaxi
Copy link
Owner

Thank you
I have created a new issue.

#160

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants