suite: fix panic if a WithStats suite skips a test early

[FGasper]

Summary

This fixes issue #1722.

Changes

Prevent stats.end() if stats.start() never ran.

Motivation

Allow skipping a test in a setup/before hook. See issue #1722 for an example.

Related issues

Closes #1722

[ccoVeille]

While it works, I would have done differently to focus the change on the small portion of code that causes the issues.

Here is the code that fails

func (s SuiteInformation) end(testName string, passed bool) {
        s.TestStats[testName].End = time.Now()
        s.TestStats[testName].Passed = passed
}

Your PR is about avoiding to call end, so to avoid the panic. But end can still panic.
A future refactoring may call this method again, and then a panic might occur again

I would like to suggest limiting the change in your PR to this

func (s SuiteInformation) end(testName string, passed bool) {
        if _, started := s.TestStats[testName]; !started {
          return
        }
        
        s.TestStats[testName].End = time.Now()
        s.TestStats[testName].Passed = passed
}

The test you added is good.

[FGasper]

@ccoVeille I myself think it’s better, if start() was never called, to avoid calling end(); thus, I’d rather the fix be as I have it.

It’s not something I feel strongly about, though, so I’ll make the change you requested.

[FGasper]

@ccoVeille Altered accordingly, with a small tweak to reduce repetition.

[ccoVeille]

@ccoVeille I myself think it’s better, if start() was never called, to avoid calling end(); thus, I’d rather the fix be as I have it.

I understand your point, thanks for considering my fix.

Good idea with that tweak

[dolmen]

@FGasper Could you rebase your branch and resolve the conflict?

[segogoreng]

Hi @dolmen
It's been 2 months, so I created a new PR, basing from @FGasper's branch and it's rebased with the latest master. Could you check that out, please? Thank you