Skip to content

feat(reporter): Add github actions reporter#9155

Closed
sidharthv96 wants to merge 5 commits into
microsoft:masterfrom
sidharthv96:feat/githubActionsAnnotations
Closed

feat(reporter): Add github actions reporter#9155
sidharthv96 wants to merge 5 commits into
microsoft:masterfrom
sidharthv96:feat/githubActionsAnnotations

Conversation

@sidharthv96
Copy link
Copy Markdown
Contributor

@sidharthv96 sidharthv96 commented Sep 26, 2021

This is a POC with minimal changes in base and a bit of duplication in logic.
I have another version where there are some changes in base to get additional data like position extracted from format functions.

Fixes: #9147
Example annotations can be found at: https://github.com/sidharthv96/jest-coverage-report-action-test/actions/runs/1275256978

TODO:

  • Unit Tests
  • Doc Updates
  • Annotate Flaky tests as warning

Needs Discussion if required:

  • Running this reporter automatically when running in Actions. (This will only report from within actions, so users could simply add it to their config too..) Land reporter first.
  • Precisely mark slow tests (Might require additional changes in base/logic duplication)

@pavelfeldman
Copy link
Copy Markdown
Member

This looks great, thank you for your patch. Some of the high level comments:

  • Please check out our other code to make sure your JS aligns with ours.
  • I don't think we should enable it based on the env for now, we can land the reporter first and enable it later
  • The amount of copy-paste for epilogue and others looks on the high side, do we have different behavior there or we could align it with the base reporter?

…sAnnotations

* upstream/master:
  browser(webkit): disable COOP support (microsoft#9185)
  fix(test runner): proper serial mode with beforeAll/afterAll failures (microsoft#9183)
  chore: remove browserType.connect from .net - not yet ready (microsoft#9182)
  test: canvas updates are reflected on screenshots (microsoft#9180)
  chore: dedupe return types in the dotnet api generator (microsoft#9181)
  docs: mention empty string in `userDataDir` (microsoft#9069)
  feat(expect): toContainText(array) (microsoft#9160)
  chore: enable object-curly-spacing in ESLint (microsoft#9168)
  feat(chromium): roll to r925110 (microsoft#9175)
  chore: render expect in trace viewer (microsoft#9141)
  devops: use Node.js 16 when rolling browsers
  docs(python): add docs about threading (microsoft#8829)
  browser(chromium): roll to r925110 (microsoft#9171)
  chore(test-runner): launch -> webServer (microsoft#9167)
  docs: cleanup test.describe.parallel.only doc (microsoft#9159)
@sidharthv96
Copy link
Copy Markdown
Contributor Author

Thanks for the review @pavelfeldman :)

Please check out our other code to make sure your JS aligns with ours.

Can you please point to which places? I've changed the interfaces to types, and used private in the class.
Apart from that, almost all of it was copied from base.

I don't think we should enable it based on the env for now, we can land the reporter first and enable it later

Sure. I added that for discussion based on #9147 (comment)

The amount of copy-paste for epilogue and others looks on the high side, do we have different behavior there or we could align it with the base reporter?

Yup, as I noted, this version is one with almost no change in base. I have a version where there are changes in base to return details like error position from the formatting functions. That would change the signature of the functions in base.
I can do that if that's ok. I could make that a separate PR so you can see the changes easily.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Add GitHub Action failed test annotation for failed tests

2 participants