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

Migrate toolbox-linter to jest #674

Open
sebastianbenz opened this issue Mar 31, 2020 · 2 comments
Open

Migrate toolbox-linter to jest #674

sebastianbenz opened this issue Mar 31, 2020 · 2 comments

Comments

@sebastianbenz
Copy link
Collaborator

sebastianbenz commented Mar 31, 2020

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

@ithinkihaveacat
Copy link
Collaborator

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.)

@sebastianbenz
Copy link
Collaborator Author

I simply want to avoid having to maintain two completely different test setups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants