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

Improved test failure display on the ScalaTest View #11

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

rlegendi
Copy link

@rlegendi rlegendi commented Aug 3, 2012

Hi all!

Hope this is the proper repo to submit any pull requests :-) I'm working on the Specs2 integration through ScalaTest plugin and got a feedback about a minor issue. I thought the best approach would be to extend the ScalaTest plugin so everyone could benefit from the patch.

The issue is that if a test if failed, its reason is not reported in any way at the ScalaTest view.

The default Eclipse JUnit runner view for instance, looks like this:

How JUnit View works

Now, the ScalaTest view had nothing similar:

How the ScalaTest view worked so far

So in this pull request I extended the view with the proper error message display that caused the test to fail:

My new cool freature

I would also welcome any code review, because after many modifications I still feel the addition a bit clumsy here and here. Thanks in advance!

I have also found an absolute library reference in the project classpath entries, changed it to relative (so after checking out the project it has no errors hopefully).

Thanks in advance!

cheeseng and others added 7 commits June 15, 2012 22:00
Conflicts:
	org.scala-ide.sdt.scalatest.tests/src/scala/tools/eclipse/scalatest/launching/ScalaTestLaunchDelegateTest.scala
	org.scala-ide.sdt.scalatest/src/scala/tools/eclipse/scalatest/launching/ScalaTestLaunchDelegate.scala
@ghost ghost assigned cheeseng Aug 13, 2012
@cheeseng
Copy link
Contributor

Hi rlegendi,

I wonder if the following commit (included in 0.9.2 that we are releasing) is trying to do the same thing as you do:-

2101732

May be instead of the 'Message: ' prefix, the exception class name is better?

@rlegendi
Copy link
Author

Hi cheeseng,

Thanks for the update! Unfortunately I've totally messed up my workspace a week ago and I wasn't able to clean it up yet :-)

On first sight, it seems the changeset you linked is indeed similar to the solution I proposed here. The only difference is that I've also updated the handleOpen() function for on-click navigation (that could work by default in your implementation).

The reason why I used the exception because Specs2 shows the failure in its message. Also, showing the exception name could be useful in some cases in my experience (like if it was an AssertionError or InstantiationError could tell you the test failed or the initialization of the test failed, e.g., it wasn't able to mock objects or something).

@cheeseng
Copy link
Contributor

cheeseng commented Jul 2, 2020

@cla-bot[bot] check

2 similar comments
@cheeseng
Copy link
Contributor

cheeseng commented Jul 3, 2020

@cla-bot[bot] check

@artimasites
Copy link

@cla-bot[bot] check

@cla-bot
Copy link

cla-bot bot commented Jul 6, 2020

Hi @rlegendi, we require contributors to sign our Contributor License Agreement, and we don't have yours on file. In order for us to review and merge your code, please access https://www.artima.com/cla/choose-type to sign our Contributor License Agreement. Your effort is highly appreciated. Thank you.

@cla-bot
Copy link

cla-bot bot commented Jul 6, 2020

The cla-bot has been summoned, and re-checked this pull request!

@plinlor
Copy link

plinlor commented Aug 10, 2020

@cla-bot[bot] check

@cla-bot
Copy link

cla-bot bot commented Aug 10, 2020

Hi @rlegendi, we require contributors to sign our Contributor License Agreement, and we don't have yours on file. In order for us to review and merge your code, please access https://www.artima.com/cla/choose-type to sign our Contributor License Agreement. Your effort is highly appreciated. Thank you.

@cla-bot
Copy link

cla-bot bot commented Aug 10, 2020

The cla-bot has been summoned, and re-checked this pull request!

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