Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(jest/circleci): setup test report for units #23851

Merged
merged 2 commits into from
May 8, 2020

Conversation

Slashgear
Copy link
Contributor

@Slashgear Slashgear commented May 6, 2020

Description

  • Generate test report on circle could help a lot contributors understand which test is broken.
  • CircleCi has a great integration of report
    • CircleCi will attach test report to execution
      • number of test executed
      • Failed tests messages (exemple here)
    • In Insight circleci tab, maintainers will be able to see global stats on tests like
      • tests that fails the most
      • longest tests

💡 This could be done with cypress test or Eslint etc

Documentation

Related Issues

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label May 6, 2020
@Slashgear Slashgear marked this pull request as ready for review May 7, 2020 20:10
@Slashgear Slashgear requested a review from a team as a code owner May 7, 2020 20:10
@Slashgear Slashgear requested a review from pieh May 7, 2020 20:10
@pvdz pvdz added status: needs core review Currently awaiting review from Core team member and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels May 8, 2020
@pieh
Copy link
Contributor

pieh commented May 8, 2020

This looks great! I will get this in, but first we want to merge #23838 because there will be conflicts in circleCi config as both of those PRs touch on test_template and it will be easier to resolve them here (much less going on than the other PR)

@Slashgear
Copy link
Contributor Author

@pieh We could consider adding this feature for other tests.
It is way simpler to see what the PR break, with report.

  • cypress tests
  • integration test

@pieh
Copy link
Contributor

pieh commented May 8, 2020

@pieh We could consider adding this feature for other tests.
It is way simpler to see what the PR break, with report.

  • cypress tests
  • integration test

Let's see how it goes with just jest unit tests for a week or so - if they prove to be trully valuable - we can enable those for other kinds of tests (I assume other kind of tests might need more involved setup as it's not as straight forward?)

Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Let's try it out

@pieh pieh merged commit 299ae64 into gatsbyjs:master May 8, 2020
@pieh
Copy link
Contributor

pieh commented May 13, 2020

Hey @Slashgear - we found some weirdness with Test summary:

For example in here - https://circleci.com/gh/gatsbyjs/gatsby/407019?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link there is failing thing - but it's not in assertions, it appears to be in setup - but it doesn't show in Test summary and also it doesn't log to "terminal output". Luckily we didn't setup test summary for windows tests ( https://circleci.com/gh/gatsbyjs/gatsby/407014?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link ) so we were able to lookup problem there - there is this in terminal output so we were able to pinpoint problem:

● Test suite failed to run

    TypeError: _opentracing.globalTracer.startSpan is not a function

      25 | const tracingContext = {
      26 |   graphqlTracing: false,
    > 27 |   parentSpan: globalTracer.startSpan(`test`),
         |                            ^
      28 | }
      29 | 
      30 | describe(`grapqhl-runner`, () => {

      at Object.startSpan (packages/gatsby/src/bootstrap/__tests__/create-graphql-runner.js:27:28)

Is it possible to setup it up to keep terminal output and not "pipe" everything to test summary? Which appears to not handle every case

@Slashgear
Copy link
Contributor Author

@pieh Jest report will only generate report for failing tests executed.
If the problem is in setup it won't appear in generated report.

But I may have missed something and removed the default test reporter.
I take it back by add

--reporters="default"

I'm on it !

@pieh
Copy link
Contributor

pieh commented May 13, 2020

I guess specifying --reporters switch replaces the default list of reporters? So it would make sense that we explicitly need to add default one back if that's the case

@pieh
Copy link
Contributor

pieh commented May 13, 2020

https://jestjs.io/docs/en/configuration#reporters-arraymodulename--modulename-options

If custom reporters are specified, the default Jest reporters will be overridden. To keep default reporters, default can be passed as a module name.

@pieh
Copy link
Contributor

pieh commented May 13, 2020

Makes total sense as CLI switch as you wouldn't want have multiple reporters outputting to terminal - it's just with Test summary when it makes less sense :P

@Slashgear
Copy link
Contributor Author

Solved it in ##24071

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs core review Currently awaiting review from Core team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants