Support from Acquia helps fund testing for Drupal Acquia logo

Comments

franskuipers’s picture

I feel its awesome we have phpunit tests in core!

+++ b/core/tests/Drupal/Tests/Component/Graph/GraphUnitTest.php
@@ -2,20 +2,20 @@
   public static function getInfo() {
     return array(

the getInfo function is not relevant in phpunit tests anymore?

+++ b/core/tests/Drupal/Tests/Component/Graph/GraphUnitTest.php
@@ -2,20 +2,20 @@
 /**
  * Unit tests for the graph handling features.
  *
  * @see Drupal\Component\Graph\Graph
  */
-class GraphUnitTest extends UnitTestBase {

add one or more @group annotations here. I am thinking of somthing like:

  * @group Component
  * @group Graph
clemens.tolboom’s picture

@msonnabaum: Why is the unit test moved? Is there some documentation about this?

See also #1938068: Convert UnitTestBase to PHPUnit

ParisLiakos’s picture

Status: Needs review » Needs work
Issue tags: +PHPUnit

Tagging

Besides

+++ b/core/tests/Drupal/Tests/Component/Graph/GraphUnitTest.phpundefined
@@ -2,20 +2,20 @@
+ * Definition of Drupal\Tests\Component\Graph\GraphUnitTest.

Should be Contains \Drupal\Tests...

this seems ready

ParisLiakos’s picture

Status: Needs work » Needs review
FileSize
8.08 KB

Status: Needs review » Needs work
Issue tags: -PHPUnit

The last submitted patch, drupal-graph_phpunit-1935908-4.patch, failed testing.

ParisLiakos’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: +PHPUnit

The last submitted patch, drupal-graph_phpunit-1935908-4.patch, failed testing.

ParisLiakos’s picture

Status: Needs work » Needs review
FileSize
8.07 KB

/me facepalms

clemens.tolboom’s picture

@ParisLiakos for future references: do you have some documentation to add in #1938068: Convert UnitTestBase to PHPUnit as most people will be puzzled how to create a patch for a git mv, replace t() by sprintf, etc ... btw: I should have documented that myself :-/

+++ b/core/tests/Drupal/Tests/Component/Graph/GraphTest.php
@@ -2,20 +2,22 @@
   public static function getInfo() {
     return array(

Shouldn't this be removed as mentioned by @franskuipers in #1?

And what about his @group Component

ParisLiakos’s picture

No, we still need it for Simpletest UI ;)
And there is no Component group, so far groups refer to a module, but definitely we should come up with a standard

clemens.tolboom’s picture

Status: Needs review » Reviewed & tested by the community

Then the patch looks ok to me :)

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

There is a clear case for using data providers in these tests

clemens.tolboom’s picture

msonnabaum’s picture

I'm not sure I see how data providers could be used here.

jhedstrom’s picture

Status: Needs work » Needs review

I also don't see a clear place to use dataproviders here.

alexpott’s picture

Yep... I saw arrays of stuff and foreach loops with assertions in... and assumed... if some rtbcs I'll commit.

jhedstrom’s picture

Status: Needs review » Reviewed & tested by the community

Moving back to RTBC as per #16.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed c43d648 and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.

jhedstrom’s picture