Add fsFixture test util #8856
Merged
Add fsFixture test util #8856
Conversation
This file contains 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
Benchmark ResultsKitchen Sink ✅
Timings
Cold BundlesNo bundle changes detected. Cached BundlesNo bundle changes detected. React HackerNews ✅
Timings
Cold Bundles
Cached Bundles
AtlasKit Editor ✅
Timings
Cold Bundles
Cached Bundles
Three.js ✅
Timings
Cold BundlesNo bundle changes detected. Cached Bundles
|
lettertwo
force-pushed
the
lettertwo/fs-fixture
branch
from
f397949
to
ea3ee72
Compare
|
@lettertwo Curious about your choice to create a DSL for this vs a typed API? e.g. fsFixture(overlayFS, __dirname)
.file(
"/commonjs/index.js",
`var local = require('./local');
// eslint-disable-next-line no-unused-vars
var url = require('url');
module.exports = function () {
return local.a + local.b;
};`
)
.file(
"/commonjs/local.js",
`exports.a = 1;
exports.b = 2;`
);A chainable API would also have the benefit of being composable via utility functions. |
Well, normally I'd recommend (to myself) against a DSL, but in this case, I think the brevity (and the narrow domain) of the DSL could be benefit. When i started thinking about this, I couldn't see an API that could improve much over: overlayFS.chdir(__dirname);
await overlayFS.mkdirp('commonjs');
await overlayFS.writeFile('commonjs/index.js', `...`);But totally open to including something if you've got more thoughts on what a builder API could look like. EDIT: Maybe you weren't actually describing a builder API, but rather just a chainable |
|
@lettertwo I think there are benefits to both approaches. I definitely wouldn't want to block your good work here, was just curious mostly. Long term I can see some extra APIs being useful as we might want to perform a similar operation across many fixtures. E.g. |
mattcompiles
reviewed
Mar 8, 2023
| type Fixture = FixtureRoot | FixtureChild; | ||
| type FixtureChild = FixtureDir | FixtureFile | FixtureLink; | ||
|
|
||
| export function fsFixture( |
There was a problem hiding this comment.
Feel like we should move this to its own package? I think this could be useful in projects outside of parcel eventually.
lettertwo
force-pushed
the
lettertwo/fs-fixture
branch
2 times, most recently
from
9024822
to
fb39c4f
Compare
lettertwo
force-pushed
the
lettertwo/fs-fixture
branch
2 times, most recently
from
62c473b
to
58af4a1
Compare
lettertwo
force-pushed
the
lettertwo/fs-fixture
branch
from
cc6c1d8
to
90c9e90
Compare
Content is no longer delimited by double quotes.
Instead, it is matched starting at a colon (:) and over subsequent lines
that have an indent level of 1 more than the filename's indent level.
tl;dr: multline line content is nested under a filename, e.g.,:
filename:
content
over multiple
lines
Previously, a root dirname like '/' could not be read because the name would be combined with another path separator, like '//'. Additionally, the directory listing would include the root directory itself.
Adds a bitmask to compute the type from the mode. Previously, files would report as both a file type and a symbolic link type.
Could be useful for porting over existing test fixtures
This uses the `path.sep` as the default `cwd`, which fixes errors on Windows like `FSError: ENOENT: \ does not exist`. Additionally, it normalizes filepaths to the fully resolved `cwd`, which on Windows resembles `C:\foo\bar`.
lettertwo
force-pushed
the
lettertwo/fs-fixture
branch
from
90c9e90
to
4a248b8
Compare
thebriando
approved these changes
Aug 16, 2023
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.
fsFixtureis test utility that is meant to make writing integration test cases easier by allowing us to colocate the filesystem-bound fixtures with the tests.Example
A test from the javascript suite converted to
fsFixtureSee the original
The test itself becomes a bit more verbose, but hopefully it's obvious that the fixtures themselves are now closer to the test.
While this example shows converting an existing test,
fsFixtureis probably most useful for authoring new tests, as it allows us to iterate on the fixtures themselves along with tests without switching between files and directories.Changes to
MemoryFSUse cases for
fsFixturerevealed a couple of edge cases that are addressed in this PR:Grammar
fsFixturedefines a simple DSL with just a handful of rules:/:->:::Since
fsFixtureis a tagged template literal, it is possible to embed expressions using the${embedded expression}syntax.See the tests in this PR for more examples!
Converting existing tests
A CLI can be found at
scripts/to-fs-fixture.jsthat is capable of doing some pretty thorough code modification to existing tests:By default, it will prompt for modifying each test it finds, and it can display a diff of the change it will apply, with options to skip, which makes running this tool on an existing test suite an incremental process.