Issue #1201 Converted test_nocasedict.py from unittest to pytest and changed test_function decorator #1237
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andy-maier
force-pushed
the
andy/#1201-nocasedict
branch
2 times, most recently
from
291e581
to
67366ef
Compare
Details: - Migrated all test cases in test_nocasedict.py from unittest to py.test. - Expanded the test cases to be quite complete. - In the (non-published) docstring of NocaseDict.update(), added information about the order of updates. This is relevant, now that NocaseDict preserves order. This is for internal clarity, as the NocaseDict description is not published. Signed-off-by: Andreas Maier <maiera@de.ibm.com>
Details: The current checking in the pytest_extensions.test_function decorator verified that the warning has been issued exactly once. In some cases (e.g. deprecated ordering comparisons for NocaseDict), the same warning message and location is issued more than once (because the NocaseDict ordering operators delegate to each other). The resulting warning in the Pythn warning system shows only one warning, because it merges warnings with the same type, message and location. However, pytest captures the warnings before that merge and thus shows more than one warning. This change relaxes the test for exactly one warning to verify that one or more warnings have been issued (of the specified type). An additional difficulty was that when a testcase specifies both an exception type and a warning type, this is really a bit overspecified, because the warning could have been issued before the exception was raised, or not. In theory, one could require that the testcases have this knowledge, but again it seems a bit overspecified. This change therefore removes the check for warnings, when an exception has also been specified in the test case. Signed-off-by: Andreas Maier <maiera@de.ibm.com>
andy-maier
force-pushed
the
andy/#1201-nocasedict
branch
from
67366ef
to
606c26e
Compare
andy-maier
changed the title
andy-maier
changed the title
KSchopmeyer
reviewed
May 1, 2018
pywbem/_nocasedict.py
Outdated
| Update the dictionary from key/value pairs. If an item for a key | ||
| exists in the dictionary, its value is updated. If an item for a key | ||
| does not exist, it is added to the dictionary at the end. | ||
| The provided keys and values are not copied. |
There was a problem hiding this comment.
This sentence takes some thinking. We mean that the values from the dictionary are used without copying them.
There was a problem hiding this comment.
DONE.
Changed the sentence to: "The provided keys and values are stored in the dictionary without being copied."
KSchopmeyer
reviewed
May 1, 2018
pywbem/_nocasedict.py
Outdated
|
|
||
| Each positional argument can be: | ||
|
|
||
| * an object with a method `items()` that returns an | ||
| :term:`py:iterable` of tuples containing key and value. | ||
| A NocaseDict or other dict object would match that requirement. |
There was a problem hiding this comment.
Don't understand this sentence
There was a problem hiding this comment.
I wanted to give a hint what this kind of positional argument is used for, but I guess it is not necessary. Removed the sentence, and also the similar sentence in the next item.
DONE.
KSchopmeyer
reviewed
May 1, 2018
pywbem/_nocasedict.py
Outdated
|
|
||
| * an object without such a method, that is an :term:`py:iterable` of | ||
| tuples containing key and value. | ||
| A list of tuples (key,value) would match that requirement. |
There was a problem hiding this comment.
Same here. Not sure what you are trying to say.
There was a problem hiding this comment.
See previous comment.
KSchopmeyer
reviewed
May 1, 2018
Signed-off-by: Andreas Maier <maiera@de.ibm.com>
|
The new commit addresses all review comments. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For details, see the commit messages (two commits).
Ready for review and merge.