print only failed tests with RerunFormatter

[furiel]

I have a distributed system, where (especially after release), I need a couple of query attempts to get the system going and produce stable responses in a timely manner. RerunFormatter significantly reduced the noise of this kind of flakiness.

However, I have some concerns with the output of the formatter, that I would like to discuss with you.

For sake of reasoning, I created a deliberately flaky scenario. This is an example output of logFailedTestCaseAttempts(), where the flaky test is retried a couple of times, but was successful in the end.

[...]
1 scenario (1 passed)
3 steps (3 passed)
0m01.992s (executing steps: 0m00.014s)
features/playground.feature:3:3:3:3:3:3:3:3:3:3

Problems with the output

1. The output contains errors, including failing steps that were eventually fixed by re-execution.
   I am leaning towards this is a bug. My primary goal was to use RerunFormatter to reduce noise. I want to see the real faliures, and not the eventially fixed flaky results. For example, in case I have a changeset I am testing, containing both actual failures and flaky ones, I cannot differentiate from the output and figure out which testcases should I investigate first.

One might argue that they want to know about flaky tests too. That's not my use case, but in case you disagree, I set the issue type to feature request.

2. The same line is repeated multiple times. This clutters the output, and make it hard to get a good grasp on how many faliures are actually. What I do is I have a separate script that post processes the output, and removes the duplicates. I don't think repeating the same lines is contains too much value.

Difficulties of extending RerunFormatter

The root of the unintended behavior is here: the guard does not consider the willBeRetried field of the steps.

I went ahead and figured maybe I can create a custom formatter extending RerunFormatter, modifying the condition to achieve what I want. But I ran into a couple of obstacles that might worth addressing, even if you want to keep the original behavior.

The plan was to copy-paste override the entire method, just with the condition changed, by adding a check for willBeRetried.

1. Aggregation and display is coupled in logFailedTestCaseAttempts()

The logic that aggregates the failures tightly coupled with printing to console. So if I want to change any behavior, I need to override the entire method. I do not have any option to get the failures, and do some post process (let's say remove duplicates from the output).

2. separator as private.

This was something I needed to work around by basically duplicating the field and it's initialization in the subclass.

What is the reason behind making it private? Could it be relaxed to protected?

3. doesNotHaveValue is not exported outside the project.

Defined here. Although these functions are fundamental to the project: most of the modules import these, none of the publicly available ones export it.

I ended up importing like

import {doesNotHaveValue} from '@cucumber/cucumber/lib/value_checker';

but I think it would worth making it publicly available.

---

This was the skeleton of the code I ended up with:

The extended class:

import {doesNotHaveValue} from '@cucumber/cucumber/lib/value_checker';

var getGherkinScenarioLocationMap = formatterHelpers.GherkinDocumentParser.getGherkinScenarioLocationMap;

function unstable(worstTestStepResult) {
  return worstTestStepResult.status !== messages.TestStepResultStatus.PASSED
}

class CustomRerunFormatter extends RerunFormatter {
  public readonly sep: string
  constructor(options: IFormatterOptions) {
    super(options);
    this.sep = options.parsedArgvOptions.rerun.separator;
  }

  logFailedTestCases(): void {
    const mapping = {}
    this.eventDataCollector
      .getTestCaseAttempts()
      .forEach(({ gherkinDocument, pickle, worstTestStepResult, willBeRetried }) => {
        if (unstable(worstTestStepResult) && !willBeRetried) {
          const relativeUri = pickle.uri
          const line =
            getGherkinScenarioLocationMap(gherkinDocument)[
              pickle.astNodeIds[pickle.astNodeIds.length - 1]
            ].line
          if (doesNotHaveValue(mapping[relativeUri])) {
            mapping[relativeUri] = []
          }
          mapping[relativeUri].push(line)
        }
      })
    const text = Object.keys(mapping)
      .map((uri) => {
        const lines = mapping[uri]
        return `${uri}:${lines.join(':')}`
      })
      .join(this.sep)
    this.log(text + "\n")
  }
}

[furiel]

I was wondering if you had the opportunity to look into this. Do these sound reasonable?

I tried to join to the slack community, but when I entered my email in the registration link I found here, I received an error about

[davidjgoss]

Hey @furiel, thanks for the detailed report and sorry for the late reply.

I do think this is a bug - for the purposes of rerun formatter we should only report a scenario as failed if all retries fail.

Since you’ve already pinpointed the source, would you be up for making a PR with the fix?

(I’ve passed in the report re the Slack invite.)

[furiel]

@davidjgoss thanks for the confirmation. I am creating a pull request.

[davidjgoss]

Fixed by #2292

Released in 9.2.0 https://github.com/cucumber/cucumber-js/releases/tag/v9.2.0

[mattwynne]

slack

@furiel for Slack, try the registration link here: https://cucumber.io/community/#slack

[furiel]

Thanks for fixing it @mattwynne . The registration worked now.