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

fix: bump deps and add wireit #72

Open
wants to merge 23 commits into
base: old-main-pnh
Choose a base branch
from
Open

Conversation

peternhale
Copy link
Collaborator

What does this PR do?

Adds wireit build tooling and bumps deps

What issues does this PR fix or reference?

@W-12558108@

@peternhale peternhale requested a review from jmsjtu March 29, 2023 21:11
@peternhale peternhale requested a review from a team as a code owner March 29, 2023 21:11
@peternhale peternhale requested review from WillieRuemmele and removed request for a team March 29, 2023 21:13
@mshanemc
Copy link

mshanemc commented Apr 6, 2023

using standard dreamhouse

❌ [fixed in QA Pr]
➜ lwc-test-qa git:(main) ../../plugin-lwc-test/bin/dev lightning:generate:lwc:test --file force-app/main/default/applications/Dreamhouse.app-meta.xml
Error (1): File must be a JavaScript file. The '.js' extension was not found: 'undefined'.

✅ setup test did what I expected
✅ run test did what I expected
-- ✅ watch mode works
-- ✅ debug mode works

❌ ❓ I can't get the PASSTHROUGH args to work.
I tried setting strict: false on the command, but that doesn't matter (running in the debugger gets the args value either way).
The problem is that jest options would be things like --runInBand or --bail which produces Error (1): Nonexistent flag: --bail
👎🏻 None of the examples show how to use the passthrough args in a valid way

../../plugin-lwc-test/bin/dev lightning:run:lwc:test bail and ../../plugin-lwc-test/bin/dev lightning:run:lwc:test -- --bail both produce the usage (help) output some other command!

Screenshot 2023-04-06 at 9 48 13 AM

lightning:run:lwc:test without project
✅ provides proper project error

Copy link

@mshanemc mshanemc left a comment

Choose a reason for hiding this comment

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

approved with non-blocking suggestions

* as arguments.
*
* The resulting argv array is then passed to the runJest method as arguments.
*/
const addArgs: string[] = [];

Choose a reason for hiding this comment

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

remove since all the intermediary mutating pushes are gone

Suggested change
const addArgs: string[] = [];
const addArgs: string[] = [];

src/commands/lightning/run/lwc/test.ts Outdated Show resolved Hide resolved
addArgs.push(args.passthrough);
// remove the '--', '--json' and 'loglevel' flags from the this.argv array
const tArgv = this.argv.filter((arg) => !['--', '--json', 'loglevel'].includes(arg));

Choose a reason for hiding this comment

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

why is json --json but loglevel isn't ?

Co-authored-by: Shane McLaughlin <m.shane.mclaughlin@gmail.com>

const hasWatchFlag = tArgv.includes('--watch');
const hasDebugFlag = tArgv.some(arg => /--debug|-d/.test(arg));

Choose a reason for hiding this comment

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

false positive for --detectopenhandles. Regex might need start/end anchors?
https://jestjs.io/docs/cli#--detectopenhandles

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.

5 participants