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

lib/ansible/parsing/ajson.py: added UNIT-tests #56398

Merged
merged 17 commits into from May 18, 2019

Conversation

Projects
None yet
4 participants
@Andersson007
Copy link
Contributor

commented May 14, 2019

SUMMARY

unit tests for lib/ansible/parsing/ajson.py AnsibleJSONEncoder

If it is OK, I'm going to add tests for other types when this PR will be merged

ISSUE TYPE
  • Bugfix Pull Request
@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

+label: bugfix

@ansibot ansibot added the bug label May 14, 2019

Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated
Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated
Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated

@ansibot ansibot added needs_revision and removed core_review labels May 15, 2019

@ansibot ansibot added core_review and removed needs_revision labels May 15, 2019

@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2019

@mattclay , AnsibleJSONEncoder is completely covered. PTAL
ready_for_review

@mattclay mattclay removed the needs_triage label May 15, 2019

Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated
Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated
Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated
Show resolved Hide resolved test/units/parsing/test_ajson.py
@mattclay

This comment has been minimized.

Copy link
Member

commented May 15, 2019

@Andersson007 Thanks for working on these tests. They're looking pretty good. Have you tried running the tests with code coverage to see if there are any gaps you can fill?

@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2019

@mattclay, thank you for reviewing! Answering your question, no, I haven’t.
I’ve never heard about it. Thank you for the link!
I’ll try to figure it out and fix all points tomorrow because unfortunately evening in my location:), then ping you

@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2019

Very good experience!

@webknjaz
Copy link
Member

left a comment

Thanks for the effort, yet I've spotted a few problems with the patch. Could you please fix those?

Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated
Show resolved Hide resolved test/units/parsing/test_ajson.py
Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated
Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated
Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated
Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated
Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated
Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated
Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated
Show resolved Hide resolved test/units/parsing/test_ajson.py Outdated

@ansibot ansibot added needs_revision and removed core_review labels May 16, 2019

@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

@webknjaz , it's good that you've come :) Best experience for me. PTAL

The most important comments have been addressed.

@ansibot ansibot added core_review and removed needs_revision labels May 16, 2019

Update test/units/parsing/test_ajson.py
Co-Authored-By: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>

Andersson007 and others added some commits May 16, 2019

Update test/units/parsing/test_ajson.py
Co-Authored-By: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>
@webknjaz
Copy link
Member

left a comment

LGTM

@webknjaz webknjaz requested a review from mattclay May 16, 2019

@webknjaz

This comment has been minimized.

Copy link
Member

commented May 16, 2019

@mattclay feel free to merge if you don't have any extra comments.

@mattclay mattclay merged commit f1b5836 into ansible:devel May 18, 2019

1 check passed

Shippable Run 123105 status is SUCCESS.
Details
@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented May 18, 2019

@webknjaz , @mattclay , thank you for reviewing!
Feel free to ping me if you need some reviews too, I’ll be glad to help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.