Don't check for address 2 presence in in person flow if the test value is nil#6575
Don't check for address 2 presence in in person flow if the test value is nil#6575
Conversation
…e is nil Currently this test renders this warning: ``` Checking for expected text of nil is confusing and/or pointless since it will always match. Please specify a string or regexp instead. /builds/lg/identity-idp/spec/features/idv/in_person_spec.rb:61 ``` ...and it does have a point. This commit adds a conditional which only runs the check if address 2 is present. Alternatively we could just remove that line entirely.
jmhooper
changed the title
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
Saw this last week as well. I think condition makes sense in case we want to specify a real value later on.
👍
spec/features/idv/in_person_spec.rb
Outdated
| expect(page).to have_text(InPersonHelper::GOOD_STATE_ID_NUMBER) | ||
| expect(page).to have_text(InPersonHelper::GOOD_ADDRESS1) | ||
| expect(page).to have_text(InPersonHelper::GOOD_ADDRESS2) | ||
| expect(page).to have_text(InPersonHelper::GOOD_ADDRESS2) if InPersonHelper::GOOD_ADDRESS2 |
There was a problem hiding this comment.
I expect constants to be ... constant? like we should know if the value exists or not ahead of time? Like we would just remove this line or no?
There was a problem hiding this comment.
Yeah, there's some indirection because this constant is ultimately derived from some other constants. But I am happy to just yank it; that makes sense too.
|
@aduth: I just switched it up to get rid of the constant, since like Zach said, we should know that it's nil if it is a constant. And I don't know if there's too much risk of trouble if it ever does assume a value. |
|
And if we're concerned about not rendering address line 2 we should probably have a unit test around that |
3 participants
Don't check for address 2 precense in in person flow if the test value is nil
Currently this test renders this warning:
...and it does have a point. This commit adds a conditional which only runs the check if address 2 is present. Alternatively we could just remove that line entirely.