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.

Sign up for GitHub

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

Jump to bottom

Added gen_html_with_total_len to generating html taking in account ta… #82

Merged
merged 6 commits into from
Oct 24, 2016
Merged

Added gen_html_with_total_len to generating html taking in account ta… #82

merged 6 commits into from
Oct 24, 2016

Conversation

renzon
Copy link

@renzon renzon commented Oct 24, 2016

…g chars

@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 99.743% when pulling 4d609b2 on renzon:master into a2848eb on omaciel:master.

1 similar comment
@coveralls
Copy link

coveralls commented Oct 24, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.008%) to 99.743% when pulling 4d609b2 on renzon:master into a2848eb on omaciel:master.

renzon pushed a commit to renzon/robottelo that referenced this pull request Oct 24, 2016
Reverting datafactory, html function moved to fauxfactory
8eab93c 
omaciel/fauxfactory#82
@rochacbruno
Copy link

@omaciel do you want to keep python 2.6 support here?

@renzon
Copy link
Author

renzon commented Oct 24, 2016

@rochacbruno Good question. I took a look on travis and the reason it's is failing on 2.6 is OderedDict use. So it's not related to this change

@rochacbruno
Copy link

@omaciel @renzon I think support for Python 2.6 can be dropped everywhere :)

@omaciel
Copy link
Owner

omaciel commented Oct 24, 2016

@rochacbruno @renzon the only reason I kept python 2.6 was because of @jlaska since he was using it for Ansible. @jlaska is this still the case?

omaciel
omaciel requested changes Oct 24, 2016
View reviewed changes
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 pending comments

fauxfactory/__init__.py

"""
if length < 8:
raise ValueError(u'Impossible generate html with len less then 7')
Copy link
Owner

Choose a reason for hiding this comment

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

"Impossible to generate HTML with len less than 7 characters"?

fauxfactory/__init__.py

random.seed()
html_tag = random.choice(HTML_TAGS)
maybe_len = length - (len(html_tag) * 2 + 5)
Copy link
Owner

Choose a reason for hiding this comment

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

Can you comment on the logic behind the calculation?

@jlaska
Copy link
Contributor

jlaska commented Oct 24, 2016

@rochacbruno @renzon the only reason I kept python 2.6 was because of @jlaska since he was using it for Ansible. @jlaska is this still the case?

I don't believe we have any requirements for running py.test on el6 environments anymore. Thanks for reaching out!

@rochacbruno
Copy link

If so, the way to go is adding the dependency https://pypi.python.org/pypi/ordereddict and make it conditional to Py2.6

@omaciel
Copy link
Owner

omaciel commented Oct 24, 2016

@rochacbruno I'm all in favor of dropping py2.6 now then :)

@rochacbruno
Copy link

@renzon Lets drop 2.6? push in your branch removing the 2.6 from tox.ini and .travia files.

@coveralls
Copy link

coveralls commented Oct 24, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-0.5%) to 99.229% when pulling 29703b9 on renzon:master into a2848eb on omaciel:master.

omaciel
omaciel requested changes Oct 24, 2016
View reviewed changes
fauxfactory/__init__.py
@@ -543,11 +551,13 @@ def gen_ipaddr(ip3=False, ipv6=False, prefix=()):
:param bool ip3: Whether to generate a 3 or 4 group IP.
:param bool ipv6: Whether to generate IPv6 or IPv4
:param list prefix: A prefix to be used for an IP (e.g. [10, 0, 1]). It
must be an iterable with strings or integers. Can be left unspecified or
must be an iterable with strings or integers. Can be left
unspecified or
empty.
Copy link
Owner

Choose a reason for hiding this comment

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

I think you can move this line up?

omaciel
omaciel reviewed Oct 24, 2016
View reviewed changes
fauxfactory/__init__.py

"""
if length < 8:
raise ValueError(u'Impossible generate html with less than 7 chars')
Copy link
Owner

Choose a reason for hiding this comment

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

Impossible to generate :)

@coveralls
Copy link

coveralls commented Oct 24, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.008%) to 99.743% when pulling 96582c9 on renzon:master into a2848eb on omaciel:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 99.743% when pulling 96582c9 on renzon:master into a2848eb on omaciel:master.

rochacbruno
rochacbruno reviewed Oct 24, 2016
View reviewed changes
tox.ini
@@ -2,7 +2,7 @@
minversion = 2.6
Copy link

Choose a reason for hiding this comment

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

This should be 2.7

@coveralls
Copy link

coveralls commented Oct 24, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-0.5%) to 99.229% when pulling bd79bd6 on renzon:master into a2848eb on omaciel:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 99.743% when pulling d4916f0 on renzon:master into a2848eb on omaciel:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 99.743% when pulling d4916f0 on renzon:master into a2848eb on omaciel:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 99.743% when pulling d4916f0 on renzon:master into a2848eb on omaciel:master.

rochacbruno
rochacbruno suggested changes Oct 24, 2016
View reviewed changes
fauxfactory/__init__.py
random.randint(0, 255) for _ in range(5)
])
random.randint(0, 255) for _ in range(5)
])
Copy link

Choose a reason for hiding this comment

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

Is the indentation ok here?

Copy link
Author

Choose a reason for hiding this comment

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

Sometimes Pycharm reformatting sucks. But at least I put the code in one line only and without using list.

@coveralls
Copy link

coveralls commented Oct 24, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.008%) to 99.743% when pulling 6fa859b on renzon:master into a2848eb on omaciel:master.

rochacbruno
rochacbruno approved these changes Oct 24, 2016
View reviewed changes
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

@omaciel omaciel merged commit 0134c89 into omaciel:master Oct 24, 2016
@rochacbruno rochacbruno mentioned this pull request Oct 26, 2016
Merged
rochacbruno pushed a commit to SatelliteQE/robottelo that referenced this pull request Oct 26, 2016
@renzon @rochacbruno
Implemented positive ui.test_myaccount.py tests (#3924)
10810b8 
* Implemented positive ui.test_myaccount.py tests

close #3862

* Changed from gen_str to gen_alpha on ui.test_myaccount.py

close #3862

* Refactored constants.LANGUAGES to a dictionary

* Reverting datafactory, html function moved to fauxfactory

omaciel/fauxfactory#82

* Moved my_account instance from MyAccountTestCase to UITestCase

* Renamed MyAccountTestCase.user to account_user
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@omaciel omaciel omaciel approved these changes

@rochacbruno rochacbruno rochacbruno approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@renzon @coveralls @rochacbruno @omaciel @jlaska