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

refactor: build test suite #1670

Merged
merged 6 commits into from Apr 15, 2022
Merged

refactor: build test suite #1670

merged 6 commits into from Apr 15, 2022

Conversation

rschristian
Copy link
Member

@rschristian rschristian commented Mar 9, 2022

What kind of change does this PR introduce?

Minor bugfixes, mostly added tests

Did you add tests for your changes?

yes

Summary

Wasn't feeling comfortable with the Webpack v5 branch, so I took to writing tests.

This fixes two very minor issues:

  • The --json flag used process.cwd(), rather than CLI's cwd
  • The Apple touch icon in the default template had a hard-coded public path of /

The build suite still skips the --cwd, --brotli, and --vebose flags.

  • --cwd powers our entire test suite, so it's a bit redundant to test
  • --brotli doesn't seem to work on a default app, not quite sure what it's supposed to do either. Static compression of assets? Might be up for removal.
  • --verbose is not particularly useful, we don't hide many logs behind that. Just debug stack traces really. Not worth testing at the moment.

I'm sure I'm still missing plenty of behaviors but this is a start, anyways.

Does this PR introduce a breaking change?

No

@changeset-bot
Copy link

changeset-bot bot commented Mar 9, 2022

🦋 Changeset detected

Latest commit: 96df7de

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
preact-cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@rschristian rschristian marked this pull request as ready for review March 9, 2022 04:38
@rschristian rschristian requested a review from a team as a code owner March 9, 2022 04:38
Comment on lines +10 to 16
try {
await symlink(
join(from, 'node_modules', name),
join(to, 'node_modules', name)
);
} catch {}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

In case this looks odd: when running multiple builds in the same test fixture (such as testing option: true & option: false together), symlinking would be triggered twice, as it's called on build. fs.symlink() throws if the symlink already exists, so we should catch & ignore those cases.

@rschristian rschristian changed the title Refactor/test suite Refactor/build test suite Mar 18, 2022
@rschristian rschristian force-pushed the refactor/test-suite branch 2 times, most recently from 49b2638 to 0f97190 Compare March 22, 2022 20:56
packages/cli/tests/lib/output.js Outdated Show resolved Hide resolved
@rschristian rschristian changed the title Refactor/build test suite refactor: build test suite Mar 29, 2022
@rschristian rschristian force-pushed the refactor/test-suite branch 2 times, most recently from 97b5bdd to 324e0c2 Compare April 2, 2022 07:18
@ForsakenHarmony ForsakenHarmony enabled auto-merge (squash) April 12, 2022 12:30
@ForsakenHarmony ForsakenHarmony merged commit 7afd8bb into master Apr 15, 2022
@ForsakenHarmony ForsakenHarmony deleted the refactor/test-suite branch April 15, 2022 00:12
@preact-bot preact-bot mentioned this pull request Apr 15, 2022
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.

None yet

2 participants