API page: https://api.drupal.org/api/drupal/core!modules!node!lib!Drupal!node!Test...

This test expects the node creation to throw an exception, but there's no visible reason why it would do that:

  function testFailedPageCreation() {
    // Create a node.
    $edit = array(
      'uid'      => $this->loggedInUser->id(),
      'name'     => $this->loggedInUser->name,
      'type'     => 'page',
      'langcode' => Language::LANGCODE_NOT_SPECIFIED,
      'title'    => 'testing_transaction_exception',
    );

    try {
      entity_create('node', $edit)->save();
      $this->fail(t('Expected exception has not been thrown.'));
    }
    catch (\Exception $e) {
      $this->pass(t('Expected exception has been thrown.'));
    }

In fact, the magic is in the node title, which a hook in the test module looks out for, and throws an exception if it spots that title.

There really should be a comment somewhere (either on the title array element, or at the try{} statement?) to say that the implementation of hook_foobar() in the test module will cause the exception to be thrown.

Without it, it's hard to understand how the test works, and also could confuse people into thinking there's something inherently wrong about the code used to create a node -- there isn't.

Comments

joachim’s picture

(Background: I needed to create a node programmatically in a test, and figured that the node test cases would be the best source of a simple example on how to do this.)

jhodgdon’s picture

Sounds reasonable...

joachim’s picture

Issue summary: View changes
andrei.dincu’s picture

Status: Active » Needs review
StatusFileSize
new725 bytes

Added
// an exception is generated if title is 'testing_transaction_exception'
// see node/tests/modules/node_test_exception/node_test_exception.module
// at hook node_test_exception_node_insert(NodeInterface $node)
as documentation.

jhodgdon’s picture

Status: Needs review » Needs work

Thanks! This is the correct documentation to add. We just need the comment to conform to our standards for how comments should be written:
https://drupal.org/node/1354#inline

The main issue is that it should be in complete sentences.

Also, I think it can be a bit shorter -- you don't really need to be so explicit about where to find the function people should look at -- you can just say "See function node_test_exception_node_insert()." in that sentence.

joachim’s picture

I agree, we don't need to be that detailed.

Even just 'An exception will be thrown by hook node_test_exception_node_insert().' would do. If you want to know more, you can follow the link to that function.

cs_shadow’s picture

Status: Needs work » Needs review
StatusFileSize
new626 bytes

Attaching patch where I've shortened the comment.

jhodgdon’s picture

Version: 8.x-dev » 7.x-dev
Status: Needs review » Patch (to be ported)

Thanks - looks good - committed to 8.x. Apparently this needs backporting.

  • Commit a172c1c on 8.x by jhodgdon:
    Issue #2244087 by cs_shadow, andrei.dincu, joachim: Add explanation...
cs_shadow’s picture

Status: Patch (to be ported) » Needs review
StatusFileSize
new484 bytes

Patch in #7 ported to 7.x

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! Looks like the right patch for 7.x.

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Thanks again! Committed to 7.x.

  • Commit f4a8b00 on 7.x by jhodgdon:
    Issue #2244087 by cs_shadow, andrei.dincu, joachim: Add explanation...

Status: Fixed » Needs work

The last submitted patch, 10: drupal-2244087-10.patch, failed testing.

jhodgdon’s picture

Status: Needs work » Fixed

Silly slow test bot. Of course the patch cannot be applied, it's already applied! :)

Status: Fixed » Closed (fixed)

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