[rfc ...] test: use verbose spec reporter

[brodybits]

No description provided.

[codecov-io]

Codecov Report

Merging #643 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #643   +/-   ##
=======================================
  Coverage   74.36%   74.36%           
=======================================
  Files          13       13           
  Lines        1845     1845           
=======================================
  Hits         1372     1372           
  Misses        473      473           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfc8177...49cf18b. Read the comment docs.

[janpio]

y?

[brodybits]

• easier to see the progress when running npm test on local workstation
• easier to understand when there is a test failure

[breautek]

I am definitely in favour of displaying stacktraces on failed tests.

[erisu]

Any thoughts on having the helper.js file like this?

if (process.env.CI) {
    const { SpecReporter } = require('jasmine-spec-reporter');

    const reporter = new SpecReporter({
        suite: {
            displayNumber: true
        },
        spec: {
            displayErrorMessages: true,
            displayStacktrace: true,
            displaySuccessful: false,
            displayFailed: true,
            displayPending: true,
            displayDuration: true
        },
        summary: {
            displayErrorMessages: false,
            displayStacktrace: false,
            displaySuccessful: false,
            displayFailed: false,
            displayPending: false,
            displayDuration: true
        }
    });

    jasmine.getEnv().clearReporters();
    jasmine.getEnv().addReporter(reporter);
}

The idea that I was thinking is we should only display the failed results and do not show any of the successful tests. This will help minimize the printout.

Additionally, I am not entirely sure if we need this type of printout formatting locally. For sure, there is a benefit in the CI testing services.

[raphinesse]

I am definitely in favour of displaying stacktraces on failed tests.

Agreed, but stacktraces for failed tests are displayed by Jasmine's default reporter as well.

The reason I enabled the verbose logging in cordova-lib was that it has some long-running tests and I wanted to see how long they take and provide better feedback about the currently executing test when running those long-running tests.

Another advantage of the verbose output is the ability to locate the origin of unwanted console output during tests.

But in case that everything goes well (test finish quickly, no garbage printed to console) I actually prefer the more terse default output when running the tests locally.

I like the solution proposed by @erisu that enables verbose output when running on CI services. Additionally, I would suggest also checking another environment variable that only controls this behavior (e.g. CDV_VERBOSE_TESTS=1) so that it can be enabled temporarily when needed locally.

However, to have all advantages that I mentioned above, we should not disable output of successful tests in verbose mode.

[erisu]

See above comments from over a month ago.

[brodybits]

Thanks to @erisu and others for the feedback. I still fail to see much benefit of reducing the verbose test output in case of running npm test on a workstation such as mine due to the amount of time I have to wait for the tests to finish. I am also not so happy about the idea of little green dots from Jasmine interspersed with the amount of console logging that comes from e2e tests.

I did make some updates to use the verbose reporter in "e2e-tests", which seems to only include a single create.spec.js test. I can think of a few more things we should consider:

• move the e2e test into its own subdirectory
• npm test should do npm run e2e-tests before the slower npm run objc-tests, which is already done on Travis CI
• npm run objc-tests seems to output a bunch of console output that is hard to follow while waiting for test results, would be nice to see this improved someday

I am keeping this PR in WIP status for now, will understand if others want to close it.

[brodybits]

I tried moving npm run objc-tests to the last place in npm test, which seems to be consistent with Travis CI, and changing the spec reporter settings as suggested in #643 (comment), definitely less noisy now. I still like the extra output when waiting for test results on my work station, oh well.