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

Unit tests for bytes_to_human #58664

Open
wants to merge 9 commits into
base: devel
from

Conversation

Projects
None yet
5 participants
@Andersson007
Copy link
Contributor

commented Jul 3, 2019

SUMMARY

Unit tests for bytes_to_human from lib/ansible/module_utils/common/text/formatters.py

ISSUE TYPE
  • Bugfix Pull Request
NEEDS TO DISCUSS

Look at the input and output from line 92-94 and 116 to 118 (the same is true for test function test_bytes_to_human_unit):

102 @pytest.mark.parametrize(
103     'input_data,unit,expected',
104     [
105         (0, u'B', u'0.00 bits'),
106         (0.5, u'B', u'0.50 bits'),
107         (0.54, u'B', u'0.54 bits'),
108         (1024, u'K', u'1.00 Kb'),
109         (1536, u'K', u'1.50 Kb'),
110         (1790, u'K', u'1.75 Kb'),
111         (1048576, u'M', u'1.00 Mb'),
112         (1099511627776, u'T', u'1.00 Tb'),
113         (1152921504606846976, u'E', u'1.00 Eb'),
114         (1180591620717411303424, u'Z', u'1.00 Zb'),
115         (1208925819614629174706176, u'Y', u'1.00 Yb'),
116         (1025, u'KB', u'1025.00 bits'),
117         (1073741824, u'Gb', u'1073741824.00 bits'),
118         (1125899906842624, u'Pb', u'1125899906842624.00 bits'),
119     ]
120 )
121 def test_bytes_to_human_unit_isbits(input_data, unit, expected):
122     """Test unit argument of bytes_to_human function with isbits=True proper results."""
123     assert bytes_to_human(input_data, isbits=True, unit=unit) == expected

Function bytes_to_human doesn't contain any docstrings.

I did grep in the source tree, the arguments isbits and unit are not used anywere.

I suggest that:

  1. we need change the logic, for example, we could check unit length and raise an exception or something else.
  2. regarding the decision about the previous point, needs to add the proper docstrings.
  3. I would try to implement the previous two points after discussion in this PR.

@Andersson007 Andersson007 force-pushed the Andersson007:unit_tests_bytes_to_human branch from 2510a8e to b0be297 Jul 3, 2019

@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2019

@samdoran @webknjaz it would be cool if you had a look

@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2019

this is the last one uncovered from formatters.py...

@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2019

ready_for_review

Update test/units/module_utils/common/text/formatters/test_bytes_to_h…
…uman.py

Co-Authored-By: Sam Doran <sdoran@redhat.com>

@Andersson007 Andersson007 changed the title [WIP] Unit tests for bytes_to_human Unit tests for bytes_to_human Jul 9, 2019

@Andersson007 Andersson007 reopened this Jul 9, 2019

@ansibot ansibot added core_review and removed WIP labels Jul 9, 2019

@ansibot ansibot added needs_revision and removed core_review labels Jul 9, 2019

@ansibot ansibot added core_review and removed needs_revision labels Jul 9, 2019

@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

ready_for_review

@bcoca bcoca removed the needs_triage label Jul 11, 2019

@samdoran samdoran self-assigned this Jul 11, 2019

@webknjaz webknjaz requested a review from samdoran Jul 12, 2019

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.