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

Added validator, tries and default to str generator functions #87

Merged
merged 1 commit into from
Mar 31, 2017

Conversation

renzon
Copy link

@renzon renzon commented Mar 15, 2017

close #86

@coveralls
Copy link

Coverage Status

Coverage increased (+0.8%) to 100.0% when pulling 9ba2dbe on renzon:86 into 87611f2 on omaciel:master.

@renzon renzon changed the title Added validator, tries and default for str generator functions Added validator, tries and default to str generator functions Mar 15, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.4%) to 97.862% when pulling 6525682 on renzon:86 into 87611f2 on omaciel:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.4%) to 97.862% when pulling 70e4deb on renzon:86 into 87611f2 on omaciel:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.4%) to 97.862% when pulling 7e8d4b5 on renzon:86 into 87611f2 on omaciel:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.4%) to 97.862% when pulling f551e94 on renzon:86 into 87611f2 on omaciel:master.

@renzon
Copy link
Author

renzon commented Mar 15, 2017

@omaciel I've tried to access coverals to check why coverage decrease but faced this msg:
Source Not Available

The owner of this repo needs to re-authorize with github; their OAuth credentials are no longer valid so the file cannot be pulled from the github API

@coveralls
Copy link

coveralls commented Mar 15, 2017

Coverage Status

Coverage increased (+0.06%) to 99.287% when pulling 22d821d on renzon:86 into 87611f2 on omaciel:master.

Copy link

@rochacbruno rochacbruno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

Comments are not blockers.


class CheckValidationTestCase(unittest.TestCase):
"""_check_validation decorator tests"""

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a real case test here:

Example is SatelliteQE/robottelo#4407 which generated string cannot start with number

And also https://github.com/SatelliteQE/robottelo/blob/master/robottelo/libvirt_discovery.py#L21 which gen_mac cannot start with fe:

(then once released we can update those codes to use validators)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't add validation to gen_mac, going to do that

if validator is None:
validator_fcn = lambda _: True
else:
validator_fcn=validator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing whitespace between operator

# Public Functions ------------------------------------------------------------


def gen_string(str_type, length=None):
def gen_string(str_type, length=None, validator=None, default=None, tries=10):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this one cannot be decorated?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once this function only delegates to already decorated functions, decorating it would only useless apply validation 2 times.

@coveralls
Copy link

coveralls commented Mar 16, 2017

Coverage Status

Coverage increased (+0.07%) to 99.297% when pulling dab3473 on renzon:86 into 87611f2 on omaciel:master.

@rochacbruno
Copy link

@omaciel can we merge this one? (and release)

Copy link
Owner

@omaciel omaciel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@omaciel omaciel merged commit 2a7523d into omaciel:master Mar 31, 2017
@omaciel
Copy link
Owner

omaciel commented Mar 31, 2017

@rochacbruno merged and published on Pypi!

@renzon it would be great to have the documentation updated to include some examples of how to use the new functionality :)

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.

add validator argument
4 participants