Skip to content

Commit

Permalink
test: add initial unit test suite (#321)
Browse files Browse the repository at this point in the history
* some unit tests

modifications made by agilgur5 to brekk's original commit:
- rebase with `master` that is 3 years newer
  - fix conflicts with newer code, esp the `tsModule`/`tsProxy` changes
    in this commit
- move `jest` config to the bottom and don't reformat `package.json`
  with tabs

* more

modifications made by agilgur5 to brekk's original commit:
- fix merge conflicts with code that is 3 years newer

* fix tests for windows

* flip that test

* deps: upgrade Jest to v28

- and ts-jest to v28
- @types/jest doesn't have a v28 yet

- look ma, no more vulns!

* refactor: split jest config to jest.config.js

- and add JSDoc typings to it
  - per https://jestjs.io/docs/configuration

* clean: remove redundant jest config

- most of these options are either unused (like `modulePaths` etc) or
  covered by Jest's defaults (`moduleFileExtensions`, `testMatch`, etc)

- also use `ts-jest` preset per its docs and migration docs
  - https://kulshekhar.github.io/ts-jest/docs/migration

* refactor: move tests into __tests__ dir

- per ezolenko's request, though I also prefer to separate them

- fix all relative imports to use `../src/` now

formatting changes:
- improve ordering of imports in tests -- externals first, newline, then
  internal imports
  - do this **consistently**
- consistently put spaces around braces in destructured imports
  - really need better linting (and update to ESLint)
- consistently add newline after imports and before declarations

- in general, add more newlines for readability in several files

* deps: import from @jest/globals instead of using globals

- no config needed and uses standard imports too
  - also required for future Jest ESM support I believe
  - I created the `jest-without-globals` package before this feature was
    released in Jest core and contributed a PR for it too, so might have
    a bias toward not using globals

* fix(test): update get-options-overrides spec to match latest file

- basically to match the changes I made when fixing declaration map
  sources ~1.5 years ago in ec0568b
- also to add `cwd` to options

- fix: use `toStrictEqual` everywhere, in order to actually check
  `undefined` properties, as this is necessary for some of the checks
  - I believe the original author may have intended to actually check
    `undefined` given that they wrote it in the test, but maybe did not
    know to use `toStrictEqual` instead of `toEqual`

* refactor(test): use obj spread in get-options-overrides

- instead of re-writing the options multiple times, write up some
  defaults and use object spread
  - and refactor `makeDefaultConfig()` to just use
    `{ ...defaultConfig }` instead to match the rest

- re-write normalizePaths to be a `for ... of` loop instead of using
  `Array.map`
  - simpler / more straightforward than a function with side-effects

- feat: add typings in several places
  - that's why we're using TS and ts-jest right??

* fix(test): update host spec to match latest source

- LanguageServiceHost now requires 3rd argument, cwd, so add that to
  all usage
- add afterDecalarations to transformers

- Jest's test.skip can now use async/await, so do that
  - it was also giving a type error, and this is simpler anyway

- change expected to use `__tests__` directory now

- fix: use `expect.arrayContaining` for `host.getDirectories` test as
  there are more temp directories that get added here, such as
  `build-self` and `coverage`
  - in general, might be less fragile if this used a generated directory
    instead of an actual one
    - but save bigger refactors for later

* refactor(test): use async/await, remove truncate func in host spec

- instead of using Promises with `done()`, use async/await for clearer
  code and less indentation too

- remove the `truncateName` function as it's not needed; `local` will
  result in the same name

- remove commented out test code
  - this seemed to just be there for testing the code, and not
    ready-for-production comments

* refactor: use consts in host spec instead of locals

- basically using pre-defined fixture vars instead of ones defined
  inside the test
  - which is more straightforward, easier to read, and less fragile
  - shorter names too

- also use a __temp dir for auto-generated files instead of creating
  them in the same dir and confusing the editor file tree and potential
  watchers
  - change jest config to make sure only spec files are being watched
  - gitignore __tests__/__temp/ dir in case tests crash etc

* fix(test): update rollupcontext spec to match latest source

- use PluginContext and IContext types instead of `any`
  - we're using TypeScript right??
  - add `debug` level logging in a few places where it was missing

- update stubbedContext to have latest PluginContext properties
  - watcher isn't in there anymore and a few new properties were added
- fix type errors with stubbedContext
  - give it an intersection with IContext for `info` and `debug`
    verbosity levels
  - force the logging funcs to `any` as they don't quite match the
    Rollup types
  - force them to `any` when deleting them as well because they're not
    optional properties

- Note: would be good to not force so much `any` if possible, but this
  may be difficult without more advanced internal Rollup mocks
  - couldn't find any testing packages for this :/

- test: add verbosity expect for debug too
- refactor: context2 -> context
  - there wasn't another one, so just use the same name consistently
    - I'm guessing there was another one at some point in the
      development of this and then it was removed but not renamed

* lint: add __tests__ to lint dirs, fix lint errors

- surprisingly only in jest.config.js?

- really need to update to @typescript-eslint ...

* ci: add unit tests to matrix

- run after all the builds for now
  - it can be done in parallel as a separate job, but then have to
    duplicate the NPM install etc, so may not be optimal that way

- refactor: add test:watch and test:coverage scripts
  - shortcut scripts so don't have to `npm test -- --coverage" etc
  - also remove `--verbose` from `test` as that's not necessary

* test: add unit tests for createFilter

- increases coverage of get-options-overrides significantly
- couldn't figure out `rootDirs` -- either the code or my tests are
  wrong, so just skip that one for now

- refactor: move makeStubbedContext etc into a shared fixtures dir so
  that it can be used for both rollupcontext tests _and_
  createFilter tests
  - in my stylistic opinion, I prefer to put nearly _all_ consts like
    these into fixtures dir
  - configure jest to ignore test helpers in coverage reporting such as
    fixture files

- format: '' -> "", add semicolon in some places
  - I use single quotes and no semicolons, so missed that in a few
    places
    - lint didn't check for that and no prettier auto-format :/

* refactor: use consts, async/await, etc in rollingcache spec

- basically using pre-defined fixture vars instead of ones defined
  inside the test
  - which is more straightforward, easier to read, and less fragile
  - shorter names too
  - left a couple of ones as is where they were only used once very
    quickly -- could make them fixture vars too but 🤷

- use async/await instead of Promises with `done()` etc
- also use more `fs-extra` functions that support Promises instead of
  synchronous `fs` functions (e.g. `existsSync` -> `pathExists`)
  - async should be a small optimization for tests too

- fix: use __temp dir for auto-generated files instead of creating
  them in a fixtures dir and breaking actual fixtures

- format: a few multi-line statements were able to be condensed to a
  single line, so do so
  - esp as multi-line was not helping readability since this is just
    irrelevant test data (may have hampered readability actually)

* docs: add a testing section to CONTRIBUTING.md

- goes over the different ways of running the tests (watch + coverage)
- line about unit and integration tests was moved into this section
  - and altered to reflect the current state of the repo now that a good
    amount of unit tests exist

- also move "Linting and Style" into its own section with a list
- move "fix any failed PR checks" to the top as overall guidance on
  making a PR
  - previously it was in the middle of linting and style, which felt a
    bit disjointed (maybe made sense earlier before builds were in CI?)

- name the last section "Building and Self-Build"; leave content as is
  - well mostly, change the first line as "fastest way to test" is not
    necessarily accurate anymore now that there are actual tests

* fix(test): undo tsTypes -> tsModules changes in source code

- original author, brekk, had made these changes, likely because without
  them, the tests would throw in the source lines where `tsModule` was
  used with things like "Cannot read property 'ModuleKind' of undefined"
  - the actual fix for this is to instead use `setTypescriptModule` from
    tsproxy as this is how it is used in the source
    - it's mainly needed for when an alternate TS is substituted
    - brekk probably didn't know the codebase well enough to know that
    - add `setTypescriptModule` to all specs that need it

- 100% test coverage of tsproxy now too!

- this should hopefully fix the build errors we were getting as well

* test: get host spec customTransformers test working

- it seemed incomplete and that's why it was skipped, no comment there
  stating otherwise, so just modified it a bit to get it to work and
  pass properly

* ci: add Node 14, 16, 18, and macOS, Windows to matrix

- drop Node 10 testing
  - Node 10 went EoL a while ago and was causing the latest version of
    Jest to throw an error
- still test Node 12 since it only went EoL recently, but could drop it
  as well if wanted
  - may want to set `package.json#engines` or update min requirements in
    the README if so, though it likely still works on older Node, we
    just can't easily test it

- add macOS and Windows testing to the matrix since TS and Rollup both
  behave differently on different OSes, in particular with regard to the
  filesystem
  - POSIX paths on Windows, case-insensitivity on macOS

- give the job a name that appears in the PR checks and Actions tab

* refactor: use __temp dir in options-overrides spec

- similar to previous commits, don't use the actual fixtures dir, use
  the `__temp` dir and a subfolder within for this spec file
  specifically so as to not interfere with other tests

* test: 100% coverage for host.ts, refactor host.spec.ts

- test: get the skipped `readFile` test to work
  - `ts.sys.readFile` definitely returns stuff, it's used for snapshots
  - it wasn't working because it was testing the wrong extension,
    `.js` instead of `.ts`
    - I thought this was intentional when I first added consts here, but
      turns out that was actually a mistake by the original author
- refactor: merge the `readFile` tests with the main test suite
  - first I merged them together, but as they were just one-liners, I
    merged them with the first set, since it wasn't particularly
    different from those

- refactor: split up the main test suite into a few commented sections
  - core snapshot functionality, fs functionality, misc
  - a lot less random now
- refactor: use `getDirectories` on the testDir instead of project root
  - much less fragile to use the dir generated here, and no hack-ish
    `arrayContaining()` either
- refactor: one-line the case-insensitive test
  - that simplifies it a lot!
- refactor: use fixed consts for the repetitive default config
  - and use a `testOpts` variable for less fragile testing

- test: add tests for version 2 branches of snapshot funcs
  - these weren't tested before so it had only tested the `|| 0` branch
    of the conditional
  - also actually test `reset` instead of calling it at the beginning
- refactor: no more "dirty checking" of instances' private interfaces
  - we're testing it all through the public interfaces instead!

- test: add a simple test for the new `trace` method

- test: coverage for `after` and `afterDeclarations` transformers
- test: coverage for different `undefined` states of transformers
  - refactor the two test suites as one for `undefined` and one for
    all 3 transformers
  - and put the working one first for naming purposes
  - refactor the `setLanguageService` call as a simpler type coercion
  - remove the transformer interactions in the main test suite

- yea, this is a big commit, but I was refactoring everything anyway and
  this is probably gonna be squashed anyway

* refactor: remove remaining "dirty checking of instance as any"

- no need to check private interfaces in these remaining tests, the
  functionality itself is already tested via the public interface

* fix(test): get tests working on Ubuntu and Windows

- host is only case-sensitive on Linux
  - prev was everything except Windows; so flipped the conditional a bit
- host and TS itself use "/" normalized paths, so use those
  - and add a hacky workaround for a TS bug I managed to find

- formatHost will use OS separators, so no "/"
  - make the test more accurate and less fragile by comparing to
    `process.cwd()` as well -- which should normalize on Windows

* refactor: use consts and shrink check-tsconfig

- think it's a bit easier to read this way, it's only 3 tests after all

* refactor: remove normalizePaths func from get-options-overrides spec

- this isn't necessary and added some complexity (and verbosity)
  - only one dir that needs normalization anymore, and that's just the
    cache dir, so just add it to the const/var fixture instead

- shrink the code a bit as a result too
  - and use a bit different code style to shrink it too

- annnd found a Windows bug in get-options-overrides.ts here too...
  - leave a `// TODO: ` comment here to fix the source as this PR is
    solely focused on tests (i.e. no source changes)

Co-authored-by: Brekk <brekk@brekkbockrath.com>
Co-authored-by: bbockrath <bbockrath@goodgamestudios.com>
  • Loading branch information
3 people committed May 10, 2022
1 parent e28d2f0 commit 56716de
Show file tree
Hide file tree
Showing 16 changed files with 10,208 additions and 1,300 deletions.
10 changes: 6 additions & 4 deletions .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ on:
branches: [ master ]

jobs:
build:

runs-on: ubuntu-latest
build-and-test:
name: Build & Test on Node ${{ matrix.node-version }} and ${{ matrix.os }}

runs-on: ${{ matrix.os }}
strategy:
matrix:
node-version: [10.x, 12.x]
node-version: [12.x, 14.x, 16.x, 18.x]
os: [ubuntu-latest, windows-latest, macOS-latest]

steps:
- uses: actions/checkout@v2
Expand All @@ -30,3 +31,4 @@ jobs:
- run: npm run build-self
env:
CI: true
- run: npm run test:coverage
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@
/build-self
/build
.rpt2_cache
*.swp
*.swo
coverage
__tests__/__temp/
22 changes: 18 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,25 @@ Attach plugin output with `verbosity` option set to 3 (this will list all files

## Developing

Use the [standard GitHub process](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/getting-started/about-collaborative-development-models#fork-and-pull-model) of forking, making a branch, and creating a PR when ready. Fix all linting problems (`npm run lint`) and fix any failed checks on the PR. Use an editor that supports [`editorconfig`](https://editorconfig.org/), or match the settings from [`.editorconfig`](./.editorconfig) manually.
Use the [standard GitHub process](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/getting-started/about-collaborative-development-models#fork-and-pull-model) of forking, making a branch, creating a PR when ready, and fixing any failing checks on the PR.

Fastest way to test changes is to do a self build; the plugin is part of its own build system:
### Linting and Style

1. Use an editor that supports [`editorconfig`](https://editorconfig.org/), or match the settings from [`.editorconfig`](./.editorconfig) manually.
1. Fix all linting problems with `npm run lint`.

### Testing

1. `npm test` to verify that all tests pass
1. `npm run test:watch` to run tests in watch mode while developing
1. `npm run test:coverage` to run tests and output a test coverage report

While this repo now has an assortment of unit tests, it still badly needs integration tests with various scenarios and expected outcomes.
Test coverage improvements for existing files and untested is needed as well.

### Building and Self-Build

One can test changes by doing a self-build; the plugin is part of its own build system.

1. make changes
1. run `npm run build` (uses last released version on npm)
Expand All @@ -28,5 +44,3 @@ Fastest way to test changes is to do a self build; the plugin is part of its own
1. run `npm run build-self` _again_ to make sure plugin built by new version can still build itself

If `build-self` breaks at some point, fix the problem and restart from the `build` step (a known good copy).

This repo badly needs unit tests and integration tests with various scenarios and expected outcomes though.
28 changes: 28 additions & 0 deletions __tests__/check-tsconfig.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { test, expect } from "@jest/globals";
import * as ts from "typescript";

import { setTypescriptModule } from "../src/tsproxy";
import { checkTsConfig } from "../src/check-tsconfig";

setTypescriptModule(ts);

const defaultConfig = { fileNames: [], errors: [], options: {} };

test("checkTsConfig", () => {
expect(() => checkTsConfig({
...defaultConfig,
options: { module: ts.ModuleKind.None },
})).toThrow(
`Incompatible tsconfig option. Module resolves to 'None'. This is incompatible with rollup, please use 'module: "ES2015"' or 'module: "ESNext"'.`,
);

expect(checkTsConfig({
...defaultConfig,
options: { module: ts.ModuleKind.ES2015 },
})).toBeFalsy();

expect(checkTsConfig({
...defaultConfig,
options: { module: ts.ModuleKind.ESNext },
})).toBeFalsy();
});
49 changes: 49 additions & 0 deletions __tests__/context.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { jest, test, expect } from "@jest/globals";

import { ConsoleContext } from "../src/context";

(global as any).console = {log: jest.fn()};

test("ConsoleContext", () => {
const proxy = new ConsoleContext(6, "=>");

proxy.warn("test");
expect(console.log).toHaveBeenLastCalledWith("=>test");

proxy.error("test2");
expect(console.log).toHaveBeenLastCalledWith("=>test2");

proxy.info("test3");
expect(console.log).toHaveBeenLastCalledWith("=>test3");

proxy.debug("test4");
expect(console.log).toHaveBeenLastCalledWith("=>test4");

proxy.warn(() => "ftest");
expect(console.log).toHaveBeenLastCalledWith("=>ftest");

proxy.error(() => "ftest2");
expect(console.log).toHaveBeenLastCalledWith("=>ftest2");

proxy.info(() => "ftest3");
expect(console.log).toHaveBeenLastCalledWith("=>ftest3");

proxy.debug(() => "ftest4");
expect(console.log).toHaveBeenLastCalledWith("=>ftest4");
});

test("ConsoleContext 0 verbosity", () => {
const proxy = new ConsoleContext(-100);

proxy.warn("no-test");
expect(console.log).not.toHaveBeenLastCalledWith("no-test");

proxy.info("no-test2");
expect(console.log).not.toHaveBeenLastCalledWith("no-test2");

proxy.debug("no-test3");
expect(console.log).not.toHaveBeenLastCalledWith("no-test3");

proxy.error("no-test4");
expect(console.log).not.toHaveBeenLastCalledWith("no-test4");
});
13 changes: 13 additions & 0 deletions __tests__/diagnostics-format-host.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { test, expect } from "@jest/globals";
import * as ts from "typescript";

import { setTypescriptModule } from "../src/tsproxy";
import { formatHost } from "../src/diagnostics-format-host";

setTypescriptModule(ts);

test("formatHost", () => {
expect(formatHost.getCurrentDirectory()).toEqual(process.cwd());
expect(formatHost.getCanonicalFileName("package.json")).toEqual("package.json");
expect(formatHost.getNewLine()).toEqual(ts.sys.newLine);
});
51 changes: 51 additions & 0 deletions __tests__/fixtures/context.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { PluginContext } from "rollup";

import { IContext } from "../../src/context";

const stub = (x: any) => x;

const contextualLogger = (data: any): IContext => {
return {
warn: (x: any) => {
data.warn = x;
},
error: (x: any) => {
data.error = x;
},
info: (x: any) => {
data.info = x;
},
debug: (x: any) => {
data.debug = x;
},
};
};

export function makeStubbedContext (data: any): PluginContext & IContext {
const { warn, error, info, debug } = contextualLogger(data);
return {
addWatchFile: stub as any,
getWatchFiles: stub as any,
cache: stub as any,
load: stub as any,
resolve: stub as any,
resolveId: stub as any,
isExternal: stub as any,
meta: stub as any,
emitAsset: stub as any,
emitChunk: stub as any,
emitFile: stub as any,
setAssetSource: stub as any,
getAssetFileName: stub as any,
getChunkFileName: stub as any,
getFileName: stub as any,
parse: stub as any,
warn: warn as any,
error: error as any,
info: info as any,
debug: debug as any,
moduleIds: stub as any,
getModuleIds: stub as any,
getModuleInfo: stub as any
};
};
151 changes: 151 additions & 0 deletions __tests__/get-options-overrides.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
import { afterAll, test, expect } from "@jest/globals";
import * as path from "path";
import * as ts from "typescript";
import { remove } from "fs-extra";

import { makeStubbedContext } from "./fixtures/context";
import { setTypescriptModule } from "../src/tsproxy";
import { IOptions } from "../src/ioptions";
import { getOptionsOverrides, createFilter } from "../src/get-options-overrides";

setTypescriptModule(ts);

const local = (x: string) => path.resolve(__dirname, x);
const cacheDir = local("__temp/get-options-overrides");

afterAll(() => remove(cacheDir));

const defaultConfig: IOptions = {
include: ["*.ts+(|x)", "**/*.ts+(|x)"],
exclude: ["*.d.ts", "**/*.d.ts"],
check: false,
verbosity: 5,
clean: false,
cacheRoot: cacheDir,
cwd: local(""),
abortOnError: false,
rollupCommonJSResolveHack: false,
typescript: ts,
objectHashIgnoreUnknownHack: false,
tsconfigOverride: null,
useTsconfigDeclarationDir: false,
tsconfigDefaults: null,
sourceMapCallback: (id: string, map: string): void => {
console.log(id + map);
},
transformers: [(ls: ts.LanguageService) => {
console.log(ls);
return {};
}],
};

const forcedOptions: ts.CompilerOptions = {
allowNonTsExtensions: true,
importHelpers: true,
inlineSourceMap: false,
moduleResolution: ts.ModuleResolutionKind.NodeJs,
noEmit: false,
noEmitHelpers: false,
noResolve: false,
outDir: `${cacheDir}/placeholder`, // TODO: fix get-options-overrides.ts on Windows by normalizing the path: https://github.com/ezolenko/rollup-plugin-typescript2/pull/321#discussion_r869710856
};

const defaultPreParsedTsConfig: ts.ParsedCommandLine = {
options: {},
fileNames: [],
errors: [],
};

test("getOptionsOverrides", () => {
const config = { ...defaultConfig };

expect(getOptionsOverrides(config)).toStrictEqual(forcedOptions);
});

test("getOptionsOverrides - preParsedTsConfig", () => {
const config = { ...defaultConfig };
const preParsedTsConfig = { ...defaultPreParsedTsConfig };

expect(getOptionsOverrides(config, preParsedTsConfig)).toStrictEqual({
...forcedOptions,
declarationDir: undefined,
module: ts.ModuleKind.ES2015,
sourceRoot: undefined,
});
});

test("getOptionsOverrides - preParsedTsConfig with options.module", () => {
const config = { ...defaultConfig };
const preParsedTsConfig = {
...defaultPreParsedTsConfig,
options: {
module: ts.ModuleKind.AMD,
},
};

expect(getOptionsOverrides(config, preParsedTsConfig)).toStrictEqual({
...forcedOptions,
declarationDir: undefined,
sourceRoot: undefined,
});
});

test("getOptionsOverrides - with declaration", () => {
const config = { ...defaultConfig, useTsconfigDeclarationDir: true };
const preParsedTsConfig = { ...defaultPreParsedTsConfig };

expect(getOptionsOverrides(config, preParsedTsConfig)).toStrictEqual({
...forcedOptions,
module: ts.ModuleKind.ES2015,
sourceRoot: undefined,
});
});

test("getOptionsOverrides - with sourceMap", () => {
const config = { ...defaultConfig };
const preParsedTsConfig = {
...defaultPreParsedTsConfig,
options: {
sourceMap: true,
},
};

expect(getOptionsOverrides(config, preParsedTsConfig)).toStrictEqual({
...forcedOptions,
declarationDir: undefined,
module: ts.ModuleKind.ES2015,
});
});

test("createFilter", () => {
const config = { ...defaultConfig };
const preParsedTsConfig = { ...defaultPreParsedTsConfig };

const stubbedContext = makeStubbedContext({});
const filter = createFilter(stubbedContext, config, preParsedTsConfig);

expect(filter("src/test.ts")).toBe(true);
expect(filter("src/test.js")).toBe(false);
expect(filter("src/test.d.ts")).toBe(false);
});

// not totally sure why this is failing
test.skip("createFilter -- rootDirs", () => {
const config = { ...defaultConfig };
const preParsedTsConfig = {
...defaultPreParsedTsConfig,
options: {
rootDirs: ["src", "lib"]
},
};

const stubbedContext = makeStubbedContext({});
const filter = createFilter(stubbedContext, config, preParsedTsConfig);

expect(filter("src/test.ts")).toBe(true);
expect(filter("src/test.js")).toBe(false);
expect(filter("src/test.d.ts")).toBe(false);
expect(filter("lib/test.ts")).toBe(true);
expect(filter("lib/test.js")).toBe(false);
expect(filter("lib/test.d.ts")).toBe(false);
});

0 comments on commit 56716de

Please sign in to comment.