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

Patch for exception message expectations #1358

Merged
merged 9 commits into from
Jul 27, 2014
Merged

Patch for exception message expectations #1358

merged 9 commits into from
Jul 27, 2014

Conversation

marcioAlmada
Copy link
Contributor

Changes:

Relates to: #1266 #1264 #1263

Things not exactly related to this PR that I think we could do better:

  • I particulary don't like the redundancy of TestCase::setExpectedException vs TestCase::setExpectedExceptionRegex, but it was the only sane way to keep the feature since there are no granular methods like TestCase::setExpectedExceptionMessage or TestCase::setExpectedExceptionCode. Any ideas?
  • The way PHPUnit grabs annotation from doc blocks is spread across multiple methods at Util\Test class, and it can be kinda tricky to expand the API (look at how much edits I had to do just to achieve a minor tweak). There are many annotations libraries around, I recommend to pick one ;). I could make a PR to improve this in the future.

@marcioAlmada marcioAlmada changed the title Patch for exception message regex Patch for exception message expectation Jul 27, 2014
@marcioAlmada marcioAlmada changed the title Patch for exception message expectation Patch for exception message expectations Jul 27, 2014
whatthejeff added a commit that referenced this pull request Jul 27, 2014
@whatthejeff whatthejeff merged commit 64efc1d into sebastianbergmann:master Jul 27, 2014
@whatthejeff
Copy link
Contributor

Thanks again, @marcioAlmada!

@marcioAlmada
Copy link
Contributor Author

Thank you too. Hope we can move to #1356 now.

@sun
Copy link
Contributor

sun commented Jul 29, 2014

Thanks a lot @marcioAlmada!

Just a minor note: Would be great to squash/fixup commits into 2-3 (next time), so as to have a clean history in the mainline. If someone needs to figure out why the code works like it does in the future, they'll highly appreciate it 😉

@marcioAlmada
Copy link
Contributor Author

@sun It tend to add a failing and a passing commit for each step I take, specially when I'm doing a PR which could potentially become a work in progress PR (when history needs to be exposed gradually), but I would have no problems if commits were squashed by someone else (@whatthejeff) since I usually delete my feature branch and pull from master later.

@marcioAlmada marcioAlmada deleted the patch/exception-message-regex branch July 31, 2014 01:27
@sebastianbergmann sebastianbergmann added this to the PHPUnit 4.3 milestone Oct 6, 2014
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.

4 participants