amp-toolbox
amp-toolbox copied to clipboard
Migrate toolbox-linter to jest
The custom setup is very hard to maintain and not very portable.
@ithinkihaveacat do you see reasons why this wouldn't work?
This also addresses #668
jest should work!
Although, I don't think there's any particular portability issues with the code itself. The "test" script in linter/package.json
is not portable, but that requires less effort to fix than migrating to jest.
How much work this is depends on how much should be migrated to jest. The existing tests have this structure:
withFixture("thumbnails2", () =>
assertMatch(
`${StoryMetadataThumbnailsAreOk.name} - publisher-logo-src missing`,
runNetworkTest(
StoryMetadataThumbnailsAreOk,
"https://regular-biology.glitch.me/"
),
"publisher-logo-src"
)
);
The three "custom" functions here are:
- withFixture() - this uses nock to set up HTTP fixtures. I think something like nock that automates the recording and reply of HTTP requests really needs to be used here, it's not feasible to wire everything up by hand. (And in many cases, the network requests happen inside other libraries like probe-image-size.) I don't know if nock can be used with jest.
-
assertMatch() - there's a few different
assert*()
functions. Should be mostly straightforward to replace with jest equivalents. -
runNetworkTest() - some initialization boilerplate that sets things up and then runs the test. (There's also a
runLocalTest()
.) The bit that does the initialization should probably be extracted.
So after these changes, the tests would look something like:
withFixture("thumbnails2", () =>
test(`${StoryMetadataThumbnailsAreOk.name} - publisher-logo-src missing`, () => {
const context = getNetworkContext("https://regular-biology.glitch.me/");
const rule = new StoryMetadataThumbnailsAreOk();
expect(rule.run(context)).toBe("publisher-logo-src");
});
);
Is this about you're expecting?
(Also, I unfortunately don't have time to work on this now.)
I simply want to avoid having to maintain two completely different test setups.