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
Fix for issue #19 (as discussed) and added generate_xml method #21
Conversation
ayushbindlish
commented
Jan 3, 2022
- Fix for Issue: generate_and_validate method does not return any json object #19
- renamed generate_and_validate method
- updated generate method with bool flag to validate
- added generate_xml method
- added tests for the above
…ol flag to validate, and added generate_xml method
Looks good, I'm currently on mobile and won't get chance to do a full review and test things out until next week sometime. Thanks for the contribution! |
Sure. Do let me know once you are able to do a full review. |
Codecov Report
@@ Coverage Diff @@
## main #21 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 13 13
Lines 383 400 +17
=========================================
+ Hits 383 400 +17
Continue to review full report at Codecov.
|
src/tests/test_default_fake.py
Outdated
@@ -229,7 +231,7 @@ def test_const(TestData): | |||
assert isinstance(f["country"], str) | |||
assert f["country"] == "United States of America" | |||
|
|||
|
|||
#@pytest.mark.xfail(reason="Always failing with "no such file or dir" error!!") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should exist if youre running the tests from a unix based machine and from the same directory as the src folder
setup.py
Outdated
@@ -12,7 +12,7 @@ | |||
long_description_content_type="text/markdown", | |||
package_dir={"": "src"}, | |||
packages=setuptools.find_packages("src", exclude=["tests"]), | |||
install_requires=["rstr", "faker", "smart_open", "jsonschema", "typer", "pydantic"], | |||
install_requires=["rstr", "faker", "smart_open", "jsonschema", "typer", "pydantic", "defusedxml", "requests", "json2xml"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since XML wont be added in the package as noted below, could you remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not work for me for some reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am on ubuntu.
command used: pytest
pwd: jsf/src
lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 20.04.3 LTS
Release: 20.04
Codename: focal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The above comments are supposed to be for pytest failure I mentioned above.
Hey, had time to get to look at this further, thanks again for the time put into this! I think I'd rather keep the xml part outside of the package as its purely for JSON and there would be no tests to ensure the functionality. I've added some inline comments, looks great! Thanks for putting this PR up and fixing issues youve found! |
Hey @ghandic I have updated the generate and validate method and removed to_xml method. |