Element attribute not exists #709
Conversation
|
I'm happy to squash these once we've sorted out the indentation etc. Can scrutiniser do this for me because I tried once already and failed miserably! |
|
According to Scrutiniser it's picking up things that aren't anything to do with this PR. |
|
yeah, this is because Scrutinizer made some changes on their side after the last run on the project, and so it finds more issues (most of them are related to a wrong phpdoc on our side, leading to Scrutinizer using the wrong type from a property because it is using this phpdoc) |
|
If the code looks alright, and there's nothing else I need to change / add, let me know and then I'll squash the commits. |
|
@stof Although I'm not one to talk (I really need to read my own PRs and issues more often, seriously) - can we get this merged in? Reason - I'm building something cool and don't want to have to host my own version of your code for my simple change. |
| @@ -982,6 +982,81 @@ public function testElementAttributeExists() | |||
| ); | |||
| } | |||
|
|
|||
| public function testElementAttributeNotExistsValid() | |||
There was a problem hiding this comment.
This test name doesn't resemble naming convention of test for elementAttributeNotExists method.
There was a problem hiding this comment.
I will rename this to testElementAttributeNotExists().
There was a problem hiding this comment.
I've expected, that you'll just copy-paste (and make changes) test for elementAttributeExists method.
|
Today’s review completed. Please fix reported issues so that I can compare added tests with existing ones. Then squash. |
|
@aik099 As you suggested, I've aggregated the tests into |
When you squash then you're set as author and as committer on commit that is merged. If I squash during merging, then you're set as author, but me as committer. If that is fine, then I'll squash during merge. |
| ->will($this->returnValue(true)) | ||
| ; | ||
|
|
||
| $this->setExpectedException('Behat\\Mink\Exception\ElementHtmlException', |
There was a problem hiding this comment.
In multi-line method/function calls each argument must be on it's own line.
|
Today’s review completed. |
|
@J7mbo , you closed this PR on purpose or just because your branch removal was pushed? |
|
@aik099 I closed it because it's 4 years old and I'm no longer working with mink. It's in my list of things I have open, so I'd like it removed from my list. Thanks! |
|
I see, no worries. Thank you. |
|
So this was never merged? I was just looking for exactly this functionality :D Here's a workaround: |
|
@JPustkuchen , that's not merged, because the PR author is not able to continue working on it. If you'd like to take over, then please do so. Make to correct all outstanding issues found in this PR as well. |
|
Thank you @aik099 I used the workaround and it works good enough for us. |
No description provided.