Page MenuHomePhabricator

[arcanist] Move jest unit tests to arc unit
AbandonedPublic

Authored by bytesofman on Jul 6 2022, 17:20.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

T2478

Experimenting with arcanist to run jest unit tests as part of arc diff

It's probable that I simply dived down the wrong path, i.e. perhaps arc unit is not the right place for this, however the ongoing support for jest-phabricator and its available docs suggests this is the production way to do this.

I've run into some issues -- this diff got the unit tests to run, and only on impacted files --- however the results are not able to be parsed by JestUnitTestEngine.php (provided by jest-phabricator documentation), leading to jest printing Pass and arcanist failing the tests.

While it is annoying to use two different systems to run automated preflight checks --- arc lint and husky --- husky is the most popular way to handle this on javascript and React apps. It also plays nice with lerna and lint-staged (https://www.npmjs.com/package/lint-staged) to target only changed files.

Importantly, husky is already working and available documentation makes it easy to maintain.

My recommendation is to continue using husky unless I'm just missing something dumb in my this arcanist attempt.

Test Plan

Change lines 4 and 5 in JestUnitTestEngine.php to reflect the correct path on your system

arc unit, note tests pass in jest but fail in arc unit

Diff Detail

Repository
rABC Bitcoin ABC
Branch
cashtab-tests-to-arc-lint
Lint
Lint Errors
SeverityLocationCodeMessage
Errorarcanist/engine/JestUnitTestEngine.php:4PHPCS.E.Generic.Files.LineLength.MaxExceededGeneric.Files.LineLength.MaxExceeded
Errorarcanist/engine/JestUnitTestEngine.php:42PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorarcanist/engine/JestUnitTestEngine.php:63PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorarcanist/engine/JestUnitTestEngine.php:64PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorarcanist/engine/JestUnitTestEngine.php:65PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorarcanist/engine/JestUnitTestEngine.php:66PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorarcanist/engine/JestUnitTestEngine.php:67PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorarcanist/engine/JestUnitTestEngine.php:68PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorarcanist/engine/JestUnitTestEngine.php:194PHPCS.E.Generic.Commenting.DocComment.ContentAfterOpenGeneric.Commenting.DocComment.ContentAfterOpen
Errorarcanist/engine/JestUnitTestEngine.php:194PHPCS.E.Generic.Commenting.DocComment.ContentBeforeCloseGeneric.Commenting.DocComment.ContentBeforeClose
Errorarcanist/engine/JestUnitTestEngine.php:194PHPCS.E.Generic.Commenting.DocComment.MissingShortGeneric.Commenting.DocComment.MissingShort
Warningarcanist/engine/JestUnitTestEngine.php:5PHPCS.W.Generic.Files.LineLength.TooLongGeneric.Files.LineLength.TooLong
Warningarcanist/engine/JestUnitTestEngine.php:182PHPCS.W.Generic.Files.LineLength.TooLongGeneric.Files.LineLength.TooLong
Warningarcanist/engine/JestUnitTestEngine.php:183PHPCS.W.Generic.Files.LineLength.TooLongGeneric.Files.LineLength.TooLong
Unit
Test Failures
Build Status
Buildable 19581
Build 38881: Build Diffcashtab-tests
Build 38880: arc lint + arc unit

Unit TestsFailed

TimeTest
0 msJavaScript tests

Event Timeline

deadalnix added inline comments.
.arcconfig
12

You want a generic engine that load the jest one, no?

.arcconfig
12

The end goal is to have arcanist run cashtab unit tests on changed files before allowing a diff to be submitted.

Right now, this is accomplished by husky, an npm module installed in Cashtab: https://www.npmjs.com/package/husky

Husky prevents git commit from committing any changes to the cashtab directory without unit tests passing.

You want a generic engine that load the jest one, no?

Looks like yes based on reviewing how lint.engine is working. I found JestUnitTestEngine in facebook's jest repo https://github.com/facebook/jest/tree/main/packages/jest-phabricator), so tried to use it here.

I'll look at implementing a generic engine that loads the jest one.

.arcconfig
12

Yes, but this is not the way, as it assume that only Jest tests would be worthy of running in there.

The jest / husky approach has been streamlined in Cashtab by other diffs. This arc unit approach would still be nice to have, but its implementation is non-trivial and relatively low impact given the effectiveness of the husky approach (standard for js repos).