Optional enhancement: Steps and/or custom test name

[danDanV1]

@garris since you like my last enhancement...

I customized my backstop helper to allow steps and custom naming. Right now ember-backstop is limited to one visual assert per test without this change.

In our app, the components we are testing can change state multiple times in a single test, eg. image placeholder, preloading animation, and loaded state. Also in-viewport detection and we need to scroll, etc. So it's much easier to test multiple steps in a single test.

    await backstop(assert, {
      step: { id: 1, name: `Rendered at scrollPos Top,Left` }
    });

    scrollIntoView(image, { block: "end", inline: "end" });

    await assertImageLoaded(image, assert);
    await backstop(assert, {
      step: { id: 2, name: `Rendered at scrollPos Bottom,Right` }
    });

or even a more fun test

  for (let index = 0; index < images.length; index++) {
    backstopScrollIntoView(images[index], true);

    await assertImageLoaded(images[index], assert);
    await backstop(assert, {
      step: { id: index, name: `Image ${index} rendered` }
    });
  }

you can even just pass a custom string

await backstop(assert, "my cool test name");

In the above, I have the custom naming a second argument and the options as a third. With a little thinking it could be merged into just one options argument.

Right now my personal API for the helper is:

await backstop(assert, "my cool test name", {scenario: { selector: ".jumbo"});

Looks a bit like this:

function createNameHash(assert, nameOptions) {
  let name;
  let testHash = {};

  // Generate base names to extend
  if (
    assert.test &&
    assert.test.module &&
    assert.test.module.name &&
    assert.test.testName
  ) {
    testHash.testId = window._testRunTime;
    testHash.scenarioId = assert.test.testId;
    name = `${assert.test.module.name} | ${assert.test.testName}`;
  } else if (assert.fullTitle) {
    // Automatic name generation for Mocha tests by passing in the `this.test` object.
    name = assert.fullTitle();
    //TODO doesn't testHash need to be set here too?
  }

  // Automatic name generation for QUnit tests by passing in the `assert` object.
  if (!nameOptions) {
    return { name: name, testHash: testHash };
  }

  // Name generation based on step.id and a name for the step.
  if (
    typeof nameOptions === "object" &&
    nameOptions.step &&
    typeof nameOptions.step.id === "number" && //a step of zero is false, so check if int.
    nameOptions.step.name
  ) {
    name = `${name} | Step #${nameOptions.step.id} | ${nameOptions.step.name}`;
    return { name: name, testHash: testHash };
  }

  // Append a unique descriptor
  if (typeof nameOptions === "string") {
    name = `${name} | ${nameOptions}`;
    return { name: name, testHash: testHash };
  }
}

/**
 * I'm in your webapps -- checkin your screenz. -schooch
 */
export default async function(assert, nameOptions, options) {
  const hash = createNameHash(assert, nameOptions);
  return new Promise((res, err) => {
    backstopHelper(hash.name, hash.testHash, options, res, err);
  }).then(backstopResult => {
    assert.ok(backstopResult.ok, backstopResult.message);
  });
}

produces:

ember-backstoptest_Integration__Component__my_component__test_name__Step_1__Image_1_rendered_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__test_name__Step_2__Image_2_rendered_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__scrolls_in_both_x_and_y_axis__Step_1__Rendered_at_scrollPos_TopLeft_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__scrolls_in_both_x_and_y_axis__Step_2__Rendered_at_scrollPos_BottomRight_0_document_0_webview.png

[garris]

Yes. This is an important use case! I see you explicitly add a step -- which is of course completely reasonable.

That said, I would like to propose that this should work automatically...

  for (let index = 0; index < images.length; index++) {
    backstopScrollIntoView(images[index], true);

    await assertImageLoaded(images[index], assert);
    await backstop(assert);
  }

and say, produce output like this...

ember-backstoptest_Integration__Component__my_component_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__Step_1_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__Step_2_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__Step_3_0_document_0_webview.png

Where the first assert is identical to current behavior and subsequent asserts are auto-incremented. This is how backstop handles selector expansion and also this counting and labeling would be more ergonomic if abstracted away.

I think it's totally reasonable to also append an assert name if supplied. This would allow your use-case where you provide this context to the end user. e.g.

await backstop(assert, {name: 'this gets appended after step ID'});

I imagine the assert object (which I believe lives for the duration of the test) could be used to track backstop() calls. Maybe this is a cleaner experience? Any thoughts?

[danDanV1]

I agree. Sounds like a cleaner API to abstract away the steps. I had the steps in there manually because I didn't know how to keep state between backstop assertions.

In that case, perhaps we could use the built-in assert.step() method, which can be used with verifySteps() or do we attach our own array to assert so we don't pollute the steps functionality. eg. track it via assert.backstop

ATM I was sketching out a solution, I started with asset.step() / assert.steps[] which is built in. Now I'm leaning towards assert.backstop[] for keeping track of the state.

[garris]

Nice. Yes -- seems like assert.backstop[] is good. Or potentially assert.backstop.steps[] is also good and reasonably future-proof. Either way -- Great!

[danDanV1]

I think I've got something that will cover it.

Before I commit the pull, there might be something I can fix too. I don't know anything about Mocha tests, but I think there might be a bug around line 90 'else if' of backstop.js helper. testHash will be sent as null in the payload. Is there another way the backstop server will acquire the testid/scenarioid? Should setting testHash be place above the entire if statement?

  // Automatic name generation for QUnit tests by passing in the `assert` object.
  if (name.test && name.test.module && name.test.module.name && name.test.testName) {
    testHash.testId = window._testRunTime;
    testHash.scenarioId = name.test.testId;
    name = `${name.test.module.name} | ${name.test.testName}`;
  } else if (name.fullTitle) {
    // Automatic name generation for Mocha tests by passing in the `this.test` object.
    name = name.fullTitle();
   //!QUESTION!! doesn't testHash need to be set here too?
  }

Because later this happens:

  const payload = JSON.stringify({
    content,
    name: name,
    widths: options.widths,
    breakpoints: options.breakpoints,
    enableJavaScript: options.enableJavaScript,
    testHash: testHash,
    origin: ORIGIN,
    scenario: options.scenario
  });

or perhaps Mocha doesn't have test.ids?

[garris]

Thanks for the attention here -- much appreciated. So I don't think we actually need the else if at all. We can just delete it since we are not specifically supporting Mocha here.

And also, we should really rename that namevariable to assert. Calling that variable name is very misleading. (I think originally this was to support passing just a string. Now I don't think we support that passing a string there.)

[danDanV1]

ha, yes, I renamed it long ago in my own backstop helper. it was confusing.

so this is what I came up with.

Automatic steps

for (let index = 0; index < images.length; index++) {
    await backstop(assert);
}

Matches the output you requested earlier:

ember-backstoptest_Integration__Component__my_component_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__Step_1_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__Step_2_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__Step_3_0_document_0_webview.png

Append a custom name to the end of the step/assertion

for (let index = 0; index < images.length; index++) {
   await backstop(assert, { name: `Image ${index} rendered` });
}

ember-backstoptest_Integration__Component__my_component_0_document__image_0_rendered_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__Step_1__image_1_rendered_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__Step_2__image_2_rendered_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__Step_3__image_3_rendered_0_document_0_webview.png

Append a custom name to the end of assertion

   await backstop(assert, { name: `did something cool` });

ember-backstoptest_Integration__Component__my_component_0_document__did_something_cool_0_document_0_webview.png

Code changes -more or less.

function createNameHash(assert, options) {
  let name = options && options.name ? options.name : ""; //optional name to append to the assertion
  let assertionName;
  let testHash = {};

  //namespace our steps to not conflict with qunit steps functionality
  if (!assert.test.backstop) assert.test.backstop = {};
  if (!assert.test.backstop.steps) assert.test.backstop.steps = [];

  let steps = assert.test.backstop.steps;

  if (name && name.length !== 0) {
    name = ` | ${name}`;
  }

  // Generate base names to extend
  if (
    assert.test &&
    assert.test.module &&
    assert.test.module.name &&
    assert.test.testName
  ) {
    testHash.testId = window._testRunTime;
    testHash.scenarioId = assert.test.testId;
    assertionName = `${assert.test.module.name} | ${assert.test.testName}`;
  }

  //Record our step
  steps.push(steps.length);

  // Do not include a step count in the first assertion
  if (steps.length === 1) {
    return { name: `${assertionName}${name}`, testHash: testHash };
  }

  // Name generation based on steps and optional name for the step
  const stepNumber = steps.length - 1;
  assertionName = `${assertionName} | Step#${stepNumber}${name}`;

  return { name: assertionName, testHash: testHash };
}

/**
 * I'm in your webapps -- checkin your screenz. -schooch
 */
export default async function(assert, options) {
  const hash = createNameHash(assert, options);
  return new Promise((res, err) => {
    backstopHelper(hash.name, hash.testHash, options, res, err);
  }).then(backstopResult => {
    assert.ok(backstopResult.ok, backstopResult.message);
  });
}

[garris]

Awesome!

Questions...

1. Have you tested this? Both with and without appending a name?
2. Should we consider placing the "name" value before the "step" value (so that if it is provided its always in the same order)?
3. Should we consider removing the "Step#" text and just include the step digit? Ok if you want to keep it -- but that hash symbol could be problematic so we should at least remove that (name handling/encoding is very crude in backstop so as a rule of thumb I've been keeping it to [a-zA-Z0-9_-]).

[danDanV1]

1. Yes, I am using the refactored helper above in my test suite in all usage scenarios.

2. Agreed to change order.

3. I don't see an issue with regex. The label will have all sorts of characters from the qunit test title and we can't control user input, and it's nice to have a easily readable label when reviewing the backstop report. I will remove the # but we should plan for the regex to sanitize.

This is how it currently renders in backstop report:

label: Integration | Component | web/item/image-single/image-only | visualRegression :: object-fit :: scroll-x | image is constrained by height | Step#1 | Image 1 rendered selector: document

filename: ember-backstoptest_Integration__Component__web_item_image-single_image-only__visualRegression__object-fit__scroll-x__image_is_constrained_by_height__Step1__Image_1_rendered_0_document_0_webview.png

But since we changing the order, and it's a bit odd to not have step0, then have step1 in the second assertion, I'd propose to rename step# to assertion and always include it.

Rendered from my test suite:

label: Integration | Component | web/item/image-single/image-only | visualRegression :: object-fit :: scroll-x | image is constrained by height | Image 1 rendered | Assertion 1 selector: document

filename: ember-backstoptest_Integration__Component__web_item_image-single_image-only__visualRegression__object-fit__scroll-x__image_is_constrained_by_height__Image_1_rendered__Assertion_1_0_document_0_webview.png

And just to match the examples we've been using await backstop(assert);

ember-backstoptest_Integration__Component__my_component__Assertion_0_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__Assertion_1_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__Assertion_2_0_document_0_webview.png

and with the custom name... await backstop(assert, { name:My cool title});

ember-backstoptest_Integration__Component__my_component__my_cool_title__Assertion_0_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__my_cool_title__Assertion_1_0_document_0_webview.png
ember-backstoptest_Integration__Component__my_component__my_cool_title__Assertion_2_0_document_0_webview.png

Question: should we count assertion# from 0 or 1?

[garris]

Thanks @edeis53! That is all reasonable -- and good point on the unpredictable characters in the test names.

Responses...

1. You can choose if we start at 1 or zero -- In other features where we auto-increment in backstop we start at zero -- but I think this implementation can have it's own rules. So -- think about what an Ember person would want to see and just do it that way. You can decide!
2. Maybe remove the underscore between assertion and the assertion value. (Your call though)
3. Maybe compress the assertion label to assert or even a (Again your call)

Thanks for your help with this other cool feature!