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

feat: add jest transformer support #6

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 24 additions & 21 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,19 @@ Additionally Speccy [doesn't support JS modules](https://github.com/wework/specc
which is a pain if you like writing your API documents like that - the underlying `oas-linter`
has no problem though.

Note that this module does not directly support YAML, but can by adding a wrapper
file that parses your YAML and linting that instead, e.g. install [yaml](https://www.npmjs.com/package/yaml)
and then create an `api.js` to lint, e.g.
Note that this module does not directly support YAML, but can support them by adding
a transformer to the jest config, e.g.

```javascript
const fs = require('fs')
const YAML = require('yaml')

const file = fs.readFileSync('./file.yml', 'utf8')
module.exports = YAML.parse(file)
module.exports = {
runner: "oas-linter",
displayName: "oas-linter",
transform: {
"\\.yaml$": "jest-yaml-transform-not-flattened"
},
testMatch: ["<rootDir>/api/*.yaml"],
moduleFileExtensions: ["yaml"]
};
Comment on lines +19 to +27
Copy link
Owner

Choose a reason for hiding this comment

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

Minor, but I prefer ' for strings and trailing commas please.

```

## Warning
Expand All @@ -30,7 +33,7 @@ This is very alpha; no tests have been written, no promises made, YMMV.

## Compatibility

This module is compatible with Node v8.x and upwards. Any incompatibilities with those versions should be reported as bugs.
This module is compatible with Node v8.x and upwards. Any incompatibilities with those versions should be reported as bugs.
Copy link
Owner

Choose a reason for hiding this comment

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

No need for copy editing :) I still haven't kicked the two-spaces-after-period habit that I was taught when younger.


## Usage

Expand All @@ -48,7 +51,7 @@ yarn add --dev jest jest-runner-oas-linter

### Add your runner to Jest config

Once you have your Jest runner you can add it to your Jest config. You will almost
Once you have your Jest runner you can add it to your Jest config. You will almost
certainly want to configure separate Jest [projects](https://jestjs.io/docs/en/configuration#projects-array-string-projectconfig)
and only use this runner on modules that export an OpenAPI document.

Expand All @@ -75,14 +78,14 @@ In your `jest-oas-linter.config.js`

```js
module.exports = {
runner: 'oas-linter',
displayName: 'oas-linter',
runner: "oas-linter",
displayName: "oas-linter",
testMatch: [
'<rootDir>/path/to/your/api/doc.js',
'<rootDir>/path/to/another/api/doc.js',
'<rootDir>/path/to/another/api/doc/**/*.js',
],
}
"<rootDir>/path/to/your/api/doc.js",
"<rootDir>/path/to/another/api/doc.js",
"<rootDir>/path/to/another/api/doc/**/*.js"
]
};
Copy link
Owner

Choose a reason for hiding this comment

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

No need to change these, leave as single quote and with trailing comma.

```

#### Minimal Steps
Expand All @@ -104,8 +107,8 @@ Or in `jest.config.js`

```js
module.exports = {
runner: 'oas-linter',
testMatch: ['<rootDir>/path/to/your/api/doc.js'],
runner: "oas-linter",
testMatch: ["<rootDir>/path/to/your/api/doc.js"]
Copy link
Owner

Choose a reason for hiding this comment

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

No need to change these, leave as single quote and with trailing comma.

};
```

Expand All @@ -115,8 +118,8 @@ Configuration can be specified in your project root in `.oaslintrc.json`, or in

Currently the only two options are to specify whether the default rules should be loaded with `loadDefaultRules` and to specify additional rules to be applied.

* `loadDefaultRules` is boolean and defaults to true, [oas-linter default rules](https://github.com/Mermade/oas-kit/blob/master/packages/oas-linter/rules.yaml) can be seen over there
* `rules` is an array of objects, as per oas-linter's [applyRules](https://github.com/Mermade/oas-kit/blob/master/packages/oas-linter/index.js#L12). More details about the format of rules supported can be found over at `oas-kit`'s [linter rules](https://mermade.github.io/oas-kit/linter-rules.html) documentation and the default rules (link above) have many examples.
- `loadDefaultRules` is boolean and defaults to true, [oas-linter default rules](https://github.com/Mermade/oas-kit/blob/master/packages/oas-linter/rules.yaml) can be seen over there
- `rules` is an array of objects, as per oas-linter's [applyRules](https://github.com/Mermade/oas-kit/blob/master/packages/oas-linter/index.js#L12). More details about the format of rules supported can be found over at `oas-kit`'s [linter rules](https://mermade.github.io/oas-kit/linter-rules.html) documentation and the default rules (link above) have many examples.
Copy link
Owner

Choose a reason for hiding this comment

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

No need for changes here.


In your `package.json`

Expand Down
49 changes: 38 additions & 11 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"dependencies": {
"create-jest-runner": "^0.5.3",
"jest": "^24.6.0",
"node-eval": "^2.0.0",
"oas-linter": "^3.0.1",
"oas-validator": "^3.3.0",
"pkg-dir": "^4.2.0"
Expand Down
31 changes: 24 additions & 7 deletions src/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const {
const validator = require('oas-validator')
const linter = require('oas-linter')
const pkgDir = require('pkg-dir')
const nodeEval = require('node-eval')

const DEFAULT_CONFIG = {
loadDefaultRules: true,
Expand All @@ -17,13 +18,13 @@ const DEFAULT_CONFIG = {

// Map an oas-linter warning to jest test result
const warningToTest = (duration, testPath) => warning => {
const { ruleName, message, pointer } = warning
const { ruleName, pointer, rule } = warning

return {
duration,
errorMessage: message, // Does not seem to be used in reporters
errorMessage: `${rule.description} on ${pointer.replace(/~1/g, '/')}`, // Does not seem to be used in reporters
testPath,
title: `${message} - ${pointer} (${ruleName})`
title: ruleName
Copy link
Owner

Choose a reason for hiding this comment

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

Can you tell me more about the changes here?

}
}

Expand Down Expand Up @@ -73,10 +74,25 @@ const loadConfig = testPath =>

// Run tests on a given file
const run = ({ testPath, config, globalConfig }) => {
const schema = require(testPath)
const { transform } = config

let schema
Copy link
Owner

Choose a reason for hiding this comment

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

No need for let here, just use const down where it's defined.

let fileContent = fs.readFileSync(testPath, { encoding: 'utf-8' })

if (
!config.transformIgnorePatterns
.map(x => new RegExp(x))
.find(x => x.test(testPath))
) {
const transformer = transform.find(x => new RegExp(x[0]).test(testPath))
Copy link
Owner

Choose a reason for hiding this comment

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

transform is not an array in the config; is this transformed by jest before the runner is called? It looks like it might have been based on your use of x[0] here for the key and x[1] below for creating the transform object; if not you might want to use Object.entries(transform).find() instead.


if (transformer) {
const transformerObj = require(transformer[1])
Copy link
Owner

Choose a reason for hiding this comment

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

It looks like transforms can be provided as an array as well, if we're going to support the jest config we should deal with that too.

fileContent = transformerObj.process(fileContent, testPath, config)
}
}

// Avoid caching for `--watch` mode
delete delete require.cache[require.resolve(testPath)]
schema = nodeEval(fileContent, testPath)
Copy link
Owner

Choose a reason for hiding this comment

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

I feel like we should be using the ScriptTransformer from @jest/transform which seems to deal with the ignore paths, permutations of how it can be configured, caching and joining of regexes, caching of transformed source. Unfortunately it's really unclear to me how we do that (or even if we need to; if we specify the transforms does jest already handle this for us or do we need to explicitly handle it like this in custom runners?)

@SimenB is there any documentation available on how to support transforms inside custom runners available? It might be a useful addition to the docs for https://github.com/jest-community/create-jest-runner.

Copy link

@SimenB SimenB Jan 15, 2020

Choose a reason for hiding this comment

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

It's up the to runner to run transforms, so you'll have to implement it here

I don't think we have any docs for it - I haven't really thought about this use case. This is untested, but should work

const { readFileSync } = require("fs");
const { ScriptTransformer } = require("@jest/transform");

const run = ({ testPath, config, globalConfig }) => {
  const content = readFileSync(testPath, "utf8");

  const scriptTransformer = new ScriptTransformer(config);

  const transformedContent =
    scriptTransformer.transformSource(testPath, content, false).code;

  // use/eval transformedContent however you'd like
};

Happy to take a PR documenting this 👍

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the comments @elyobo, I think most of the issues come from vscode formatting the files on save. Do you have an editor config or .prettierrc that I could use to format everything properly?


const test = {
path: testPath,
Expand Down Expand Up @@ -123,7 +139,7 @@ const run = ({ testPath, config, globalConfig }) => {
return resolve(
fail({
...result,
errorMessage: 'Not a valid OpenAPI schema.'
errorMessage: 'Not a valider OpenAPI schema: ' + error.message
Copy link
Owner

Choose a reason for hiding this comment

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

Use a template string.

Suggested change
errorMessage: 'Not a valider OpenAPI schema: ' + error.message
errorMessage: `Not a valider OpenAPI schema: ${error.message}`,

})
)
}
Expand All @@ -133,6 +149,7 @@ const run = ({ testPath, config, globalConfig }) => {
toTestResult({
errorMessage:
'Schema is valid, but linting errors were present.',
skipped: false,
stats: {
failures: warnings.length,
pending: 0,
Expand Down