Improve isFieldInResponse to handle more combinations - #35618
Conversation
Codecov Report
@@ Coverage Diff @@
## master #35618 +/- ##
=============================================
- Coverage 65.68% 48.9% -16.79%
=============================================
Files 1222 109 -1113
Lines 70932 10569 -60363
Branches 1289 1289
=============================================
- Hits 46594 5169 -41425
+ Misses 23960 5022 -18938
Partials 378 378
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #35618 +/- ##
=============================================
- Coverage 65.68% 48.9% -16.79%
=============================================
Files 1222 109 -1113
Lines 70932 10569 -60363
Branches 1289 1289
=============================================
- Hits 46594 5169 -41425
+ Misses 23960 5022 -18938
Partials 378 378
Continue to review full report at Codecov.
|
| * @return bool | ||
| */ | ||
| public function isFieldInResponse($field, $contentExpected, $data = null) { | ||
| public function isFieldInResponse($field, $contentExpected, $expectSuccess = true, $data = null) { |
There was a problem hiding this comment.
Note: nothing currently calls this passing $data parameter. So we can push it out to parameter 4 without breaking anything.
There was a problem hiding this comment.
That's really not the $expectSuccess, but provides debug information (so, something like $debug = true?). And, as it's purpose is to provide debug info, it would be good to push it to the edge, I think.
There was a problem hiding this comment.
It does actually tell the function if the caller is expecting a true or false return.
If the caller is expecting true but the function is about to return false then emit some debug info to give clues about why the expected content was not found.
If the caller is expecting false but the function is about to return true then emit some debug info to give clues about what (unexpected) content was found.
| return (\strlen((string)$element->$field) == 15); | ||
| $fieldIsSet = true; | ||
| $fieldValue = (string)$element->$field; | ||
| if ($contentExpected == "ANY_VALUE") { |
There was a problem hiding this comment.
| if ($contentExpected == "ANY_VALUE") { | |
| if ($contentExpected === "ANY_VALUE") { |
There was a problem hiding this comment.
@individual-it I did another refactoring, also putting in plenty of ===
Please review the whole thing again.
76b2a71 to
6f71e04
Compare
| public function doesFieldValueMatchExpectedContent( | ||
| $field, $value, $contentExpected, $expectSuccess = true | ||
| ) { | ||
| if (($contentExpected === "ANY_VALUE") |
There was a problem hiding this comment.
It was quite difficult to understand previously. It's readable now. Next thing to do would be to not pass $contentExpected as string (except, for if we are checking for value). We could encapsulate these on something like Token class as static variables.
There was a problem hiding this comment.
or using our %something% inline substitutions
There was a problem hiding this comment.
Yes, someday this will need the next refactoring.
e.g. if/when somebody writes a test with usernames A_TOKEN and A_NUMBER and they want to test that share_with field has the actual value A_NUMBER :)
6f71e04 to
0496af1
Compare
| ) . " 00:00:00"; | ||
| } | ||
|
|
||
| $contentExpected = (string) $contentExpected; |
There was a problem hiding this comment.
We let the caller pass "mixed" in $contentExpected (just because they can anyway).
The responses really only have string values (even if they happen to look like numbers...). So we cast to string here, then can do === comparisons in comfort.
0496af1 to
afb7d3a
Compare
|
Backport |
4 participants
Description
Enhance
isFieldInResponse()acceptance test method so that it:ANY_VALUEso we can test for a field existing (or not existing) and we do not care what the value is.Motivation and Context
If I write:
Then if the
share_withfield exists, but has some other value likebob, then the test passes.I want to be able to to test that a field does not exist at all.
If the field is not as expected, then as well as a fail message, I would like to know what value it was set to, or if the field did not exist. That can really help when trying to work out why a test is failing.
How Has This Been Tested?
Local runs of
createShare.featurescenarios with different combinations of fields and values in steps for:to verify that when something is different from expected, that the test does fail, and emits a somewhat-useful message.
Types of changes
Checklist:
Open tasks: