-
Notifications
You must be signed in to change notification settings - Fork 360
fix: Resolve entries against cwd #1091
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
Open
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
⚠️ No Changeset found
Latest commit: 0485a4c
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
Contributor
|
Size Change: 0 B Total Size: 65.9 kB i️ View Unchanged
|
@rschristian
rschristian
force-pushed
the
fix/glob-entry
branch
2 times, most recently
from
October 24, 2025 02:51
c4ef5dc to
4167d53
Compare
@rschristian
rschristian
force-pushed
the
fix/glob-entry
branch
from
October 24, 2025 02:53
4167d53 to
0485a4c
Compare
Comment on lines
+59
to
+62
// Now that dirs have been cleared, reset the fixturePath back to the parent dir
if (fixtureDir.endsWith('-with-cwd')) {
fixturePath = resolve(FIXTURES_DIR, fixtureDir);
}
Collaborator
Author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was super broken & went unnoticed for a long time.
Basically we make the assumption that a test fixture foo-with-cwd is testing --cwd ./foo, and to make sure we were clearing out past runs, set the fixture path to fixtures/foo-with-cwd/foo so that rimraf would clear out all test cases the same way. However, we never set this path back!
When it came time to get the build script & execute it, the CWD was /foo-with-cwd/foo... none of these nested directories actually had a build script defined, as it was meant to come from the parent, and so all of the -with-cwd tests were just running plain microbundle. The build script we provided for our tests was entirely ignored as the test was always ran from the incorrect working dir.
Easy fix though, we just have to reset it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
preactjs/preact#4677
tiny-globresolves againstopts.cwdor., and because we don't passcwdintotiny-glob, this can result in incorrect file paths when used with Microbundle's--cwdflag.Edit: Meant to open this as a draft, needs tests &
(削除) ideally errors thrown for missing inputs (削除ここまで)Edit2: Unfortunately we don't throw on missing inputs anywhere else so that'd be inconsistent.