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

[BUGFIX] Fix first-child and last-child selectors #257

Merged
merged 1 commit into from
Oct 12, 2015
Merged

Conversation

oliverklee
Copy link
Contributor

Fixes #51
Fixes #192

@jjriv
Copy link
Contributor

jjriv commented Oct 12, 2015

-The following selectors are implemented, but currently are broken:
-
  * first-child (currently broken)
  * last-child (currently broken)

I'm confused. Are these selectors still broken or not? We are removing the comment saying they are broken, and the PR says this bug is getting fixed, but inside the parens it still says "currently broken".

@jjriv
Copy link
Contributor

jjriv commented Oct 12, 2015

-            $containedHtml,
-            $this->subject->emogrify()
+            $htmlRegularExpression,
+            $result
         );```

According to the docs, $result should be a string:
https://phpunit.de/manual/current/en/appendixes.assertions.html#appendixes.assertions.assertRegExp

But I don't see where $result is getting assigned any value before being passed to assertRegExp()

@oliverklee
Copy link
Contributor Author

I've just removed the two "(currently broken)" from the docs. I'll look into the $result thing now.

@oliverklee
Copy link
Contributor Author

$result is string because it gets assigned the return value of $this->subject->emogrify().

And I put it into a local variable to separate the execution from the verification according to the four-phase test concept: https://robots.thoughtbot.com/four-phase-test

jjriv added a commit that referenced this pull request Oct 12, 2015
[BUGFIX] Fix first-child and last-child selectors
@jjriv jjriv merged commit 4bd6ed4 into master Oct 12, 2015
@jjriv jjriv deleted the bugfix/nth-child branch October 12, 2015 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants