test: add initial integration test suite (#371)

* test: add initial integration test harness

- some additional ts-jest config is needed to parse `index.ts`
  - since Rollup isn't used when importing files during testing, we need to enable `esModuleInterop`
    - the import of `findCacheDir` was immediately erroring without this (and maybe more, but `esModuleInterop` made the import work)
  - add `tsconfig.test.json` for this purpose
  - use `ts-jest` JSDoc types to get types for the ts-jest config as well

- add an integration/ dir in the __tests__/ dir for all integration tests
  - add a fixtures/ dir in there as well for integration test fixtures
    - add a `tsconfig.json` for all fixtures to use
      - basically so that rpt2 source code is not `include`d, though also bc this may be good to customize for fixtures
    - add a simple fixture with "no-errors" that does an import and a type-only import

- add a simple integration test that just ensures that all files are part of the bundle
  - the bundled `index` file and each file's declaration and declaration map
  - and no _other_ files!
  - for Rollup, need to use paths relative to the root of the whole project
    - probably because that is `cwd` when running `jest` from root
    - will probably abstract out helpers here in the future as the test suite grows
  - 70% coverage of `index.ts` with just this one test!

- update CONTRIBUTING.md now that an integration test exists

* refactor: use `local` func for path resolution

- similar to the unit test suite

- also remove the `.only` that I was using during development

* add comparison test for cache / no cache

- should probably do this for each integration test suite

- refactor: split out a `genBundle` func to DRY things up
  - this was bound to happen eventually
    - don't think testing other output formats is necessary since we don't have any specific logic for that, so that is just Rollup behavior
  - take `input` path and rpt2 options as params
    - and add the `tsconfig` as a default
  - add a hacky workaround so that Rollup doesn't output a bunch of warnings in the logs
  - format: use double quotes for strings consistently (my bad, I normally use single quotes in my own repos)

* refactor: use a temp cacheRoot

- so we can clean it up after testing to ensure consistency
  - and so we're not filling up the cache in `node_modules` with testing caches

- also add a comment to one the of the tests

* test: check w/ and w/o declarations + declaration maps

- make sure they don't get output when not asked for

* fix(test): actually test the cache

- `clean: true` also means that no cache is created
  - which is a bit confusing naming, but was requested by a few users and implemented in f15cb84
  - so have to create the bundle one _more_ time to create the cache
    - note that `tscache`'s `getCached` and `isDirty` functions are now actually covered in the coverage report

* fix(ci): increase integration test timeout

- double it bc it was occassionally failing on CI due to timeouts

* test: ensure that JS files are part of bundle too

- ensures that TS understands the JS w/ DTS w/o error
- and that rpt2 filters out JS while Rollup still resolves it on its own (since Rollup understands ESM)
  - similar to testing w/ a different plugin (i.e. literally testing an "integration"), but this is native Rollup behavior in this case where it doesn't need a plugin to understand ESM
- also the first test of actual code contents

- reformat the cache test a bit into its own block since we now have ~3 different sets of tests in the suite

* optim(test): don't need to check cache each time

- this doesn't test a new code path and the first test already tests the entire bundle for the fixture, so the other tests just repeat that

* test: add initial error checking integration tests

- refactor: rename `integration/index.spec` -> `integration/no-errors.spec`
- refactor: split `genBundle` func out into a helper file to be used by multiple integration test suites
  - simplify the `genBundle` within `no-errors` as such

- create new `errors` fixture just with some simple code that doesn't type-check for now
  - add test to check that the exact semantic error pops up
    - and it even tests colors 😮  ...though I was confused for some time why the strings weren't matching... 😐
  - add test to make sure it doesn't pop up when `check: false` or `abortOnError: false`
    - when `abortOnError: false`, detect that a warning is created instead

- due to the new `errors` dir in the fixtures dir, the fixtures `tsconfig` now picks up two dirs
  - which changes the `rootDir` etc
  - so create two tiny `tsconfig`s in both fixture dirs that just extend that one
    - and now tests all run simiarly to before

* fix(ci): increase integration test `errors` timeout

- this too timed out, probably due to the checks that _don't_ error

* optim(test): split non-erroring error tests into a different suite

- so that Jest can parallelize these
  - CI was timing out on this, so splitting it out should help as it'll be 10s each

* fix(ci): bump integration test timeout to 15s

- bc it was still timing out in some cases 😕
  - this time the `no-errors` suite was timing out, so just increase all around

* fix(test): output a `.js` bundle, not `.ts`

- woooops... was wondering why it was `.ts`; turns out because I wrote the `output.file` setting that way 😅 😅 😅

- also add a missing comment in errors for consistency
- and put code checks _after_ file structure checks, since that's a deeper check

* test: check that `emitDeclarationOnly` works as expected

- should output declaration and declaration map for file
  - code + declaration should contain the one-line function
  - code _shouldn't_ contain anything from TS files

- since this is plain ESM code, we don't need another plugin to process this
  - nice workaround to installing another plugin that I hadn't thought of till now!

* test: add a syntactic error, refactor a bit

- add a file to `errors` with a syntax error in it
  - apparently this does throw syntax err, but does not cause `emitSkipped: true`... odd...
    - so may need another test for that...
    - `abortOnError: false` / `check: false` both cause Rollup to error out instead
  - rename `errors/index` -> `errors/semantic` since we have different kinds now
  - change the `include` to only take a single file, so that we don't unnecessarily generate declarations or type-check other error files

- refactor(test): rewrite `genBundle` in both files to take a relative path to a file
  - simplifies the `emitDeclarationOnly` test as we can now just reuse the local `genBundle` instead of calling `helpers.genBundle` directly
  - use the same structure for `errors`'s different files as well

- refactor(test): split `errors.spec` into `tsconfig` errors, semantic errors, and syntactic errors (for now)

- add a slightly hacky workaround to get `fs.remove` to not error
  - seems to be a race condition due to the thrown errors and file handles not being closed immediately on throw (seems like they close during garbage collection instead?)
  - see linked issue in the comment; workaround is to just give it some more time
    - not sure that there is a true fix for this, since an improper close may cause indeterminate behavior

* fix(test): normalize as arguments to `include`

- got a Windows error in CI for the `errors` test suite

Author: agilgur5

.gitignore

@@ -9,4 +9,4 @@
 *.swp
 *.swo
 coverage
-__tests__/__temp/
+__temp

CONTRIBUTING.md

@@ -30,8 +30,8 @@ Use the [standard GitHub process](https://docs.github.com/en/pull-requests/colla
 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.
+While this repo now has an assortment of unit tests and integration tests, it still needs more integration tests with various scenarios and expected outcomes.
+Test coverage improvements for existing files and untested files is needed as well.
 
 ### Building and Self-Build
 
@@ -72,7 +72,7 @@ A useful resource as you dive deeper are the [unit tests](__tests__/). They're g
       - [Using the Language Service API](https://github.com/microsoft/TypeScript/wiki/Using-the-Language-Service-API)
       - _NOTE_: These are fairly short and unfortunately leave a lot to be desired... especially when you consider that this plugin is actually one of the simpler integrations out there.
 1. At this point, you may be ready to read the more complicated bits of [`index`](src/index.ts) in detail and see how it interacts with the other modules.
-    - The integration tests [TBD] could be useful to review at this point as well.
+    - The [integration tests](__tests__/integration/) could be useful to review at this point as well.
 1. Once you're pretty familiar with `index`, you can dive into some of the cache code in [`tscache`](src/tscache.ts) and [`rollingcache`](src/rollingcache.ts).
 1. And finally, you can see some of the Rollup logging nuances in [`context`](src/context.ts) and [`rollupcontext`](src/rollupcontext.ts), and then the TS logging nuances in [`print-diagnostics`](src/print-diagnostics.ts), and [`diagnostics-format-host`](src/diagnostics-format-host.ts)
     - While these are necessary to the implementation, they are fairly ancillary to understanding and working with the codebase.

__tests__/integration/errors.spec.ts

@@ -0,0 +1,66 @@
+import { jest, afterAll, test, expect } from "@jest/globals";
+import * as path from "path";
+import { normalizePath as normalize } from "@rollup/pluginutils";
+import * as fs from "fs-extra";
+import { red } from "colors/safe";
+
+import { RPT2Options } from "../../src/index";
+import * as helpers from "./helpers";
+
+// increase timeout to 15s for whole file since CI occassionally timed out -- these are integration and cache tests, so longer timeout is warranted
+jest.setTimeout(15000);
+
+const local = (x: string) => path.resolve(__dirname, x);
+const cacheRoot = local("__temp/errors/rpt2-cache"); // don't use the one in node_modules
+const onwarn = jest.fn();
+
+afterAll(async () => {
+  // workaround: there seems to be some race condition causing fs.remove to fail, so give it a sec first (c.f. https://github.com/jprichardson/node-fs-extra/issues/532)
+  await new Promise(resolve => setTimeout(resolve, 1000));
+  await fs.remove(cacheRoot);
+});
+
+async function genBundle(relInput: string, extraOpts?: RPT2Options) {
+  const input = normalize(local(`fixtures/errors/${relInput}`));
+  return helpers.genBundle({
+    input,
+    tsconfig: local("fixtures/errors/tsconfig.json"),
+    cacheRoot,
+    extraOpts: { include: [input], ...extraOpts }, // only include the input itself, not other error files (to only generate types and type-check the one file)
+    onwarn,
+  });
+}
+
+test("integration - tsconfig errors", async () => {
+  // TODO: move to parse-tsconfig unit tests?
+  expect(genBundle("semantic.ts", {
+    tsconfig: "non-existent-tsconfig",
+  })).rejects.toThrow("rpt2: failed to open 'undefined'"); // FIXME: bug: this should be "non-existent-tsconfig", not "undefined"
+});
+
+test("integration - semantic error", async () => {
+  expect(genBundle("semantic.ts")).rejects.toThrow(`semantic error TS2322: ${red("Type 'string' is not assignable to type 'number'.")}`);
+});
+
+test("integration - semantic error - abortOnError: false / check: false", async () => {
+  // either warning or not type-checking should result in the same bundle
+  const { output } = await genBundle("semantic.ts", { abortOnError: false });
+  const { output: output2 } = await genBundle("semantic.ts", { check: false });
+  expect(output).toEqual(output2);
+
+  expect(output[0].fileName).toEqual("index.js");
+  expect(output[1].fileName).toEqual("semantic.d.ts");
+  expect(output[2].fileName).toEqual("semantic.d.ts.map");
+  expect(output.length).toEqual(3); // no other files
+  expect(onwarn).toBeCalledTimes(1);
+});
+
+test("integration - syntax error", () => {
+  expect(genBundle("syntax.ts")).rejects.toThrow(`syntax error TS1005: ${red("';' expected.")}`);
+});
+
+test("integration - syntax error - abortOnError: false / check: false", () => {
+  const err = "Unexpected token (Note that you need plugins to import files that are not JavaScript)";
+  expect(genBundle("syntax.ts", { abortOnError: false })).rejects.toThrow(err);
+  expect(genBundle("syntax.ts", { check: false })).rejects.toThrow(err);
+});

__tests__/integration/fixtures/errors/semantic.ts

@@ -0,0 +1,3 @@
+export const sum = (a: number, b: number): number => {
+  return "a + b";
+}

__tests__/integration/fixtures/errors/syntax.ts

@@ -0,0 +1,3 @@
+export const incorrectSyntax => {
+  return "a + b";
+}

__tests__/integration/fixtures/errors/tsconfig.json

@@ -0,0 +1,3 @@
+{
+  "extends": "../tsconfig.json",
+}

__tests__/integration/fixtures/no-errors/index.ts

@@ -0,0 +1,8 @@
+export function sum(a: number, b: number) {
+  return a + b;
+}
+
+export { difference } from "./some-import"
+export type { num } from "./type-only-import"
+
+export { identity } from "./some-js-import"

__tests__/integration/fixtures/no-errors/some-import.ts

@@ -0,0 +1,3 @@
+export function difference(a: number, b: number) {
+  return a - b;
+}

__tests__/integration/fixtures/no-errors/some-js-import.d.ts

@@ -0,0 +1,4 @@
+// TS needs a declaration in order to understand the import
+// but this is ambient, and so should not be directly imported into rpt2, just found by TS
+
+export function identity(a: any): any;

__tests__/integration/fixtures/no-errors/some-js-import.js

@@ -0,0 +1,5 @@
+// should be filtered out by rpt2, but still bundled by Rollup itself (as this is ESM, no need for a plugin)
+
+export function identity(a) {
+  return a;
+}

__tests__/integration/fixtures/no-errors/tsconfig.json

@@ -0,0 +1,3 @@
+{
+  "extends": "../tsconfig.json",
+}

__tests__/integration/fixtures/no-errors/type-only-import.ts

@@ -0,0 +1 @@
+export type num = number;

__tests__/integration/fixtures/tsconfig.json

@@ -0,0 +1,11 @@
+{
+	"compilerOptions": {
+    "forceConsistentCasingInFileNames": true,
+    "moduleResolution": "node",
+    "sourceMap": true,
+		"declaration": true,
+		"declarationMap": true,
+    "target": "ES6",
+    "strict": true,
+	},
+}

__tests__/integration/helpers.ts

@@ -0,0 +1,41 @@
+import { rollup, RollupOptions, OutputAsset } from "rollup";
+
+import rpt2, { RPT2Options } from "../../src/index";
+
+type Params = {
+  input: string,
+  tsconfig: string,
+  cacheRoot: string,
+  extraOpts?: RPT2Options,
+  onwarn?: RollupOptions['onwarn'],
+};
+
+export async function genBundle ({ input, tsconfig, cacheRoot, extraOpts, onwarn }: Params) {
+  const bundle = await rollup({
+    input,
+    plugins: [rpt2({
+      tsconfig,
+      cacheRoot,
+      ...extraOpts,
+    })],
+    onwarn,
+  });
+
+  const esm = await bundle.generate({
+    file: "./dist/index.js",
+    format: "esm",
+    exports: "named",
+  });
+
+  // Rollup has some deprecated properties like `get isAsset`, so enumerating them with, e.g. `.toEqual`, causes a bunch of warnings to be output
+  // delete the `isAsset` property for (much) cleaner logs
+  const { output: files } = esm;
+  for (const file of files) {
+    if ("isAsset" in file) {
+      const optIsAsset = file as Partial<Pick<OutputAsset, "isAsset">> & Omit<OutputAsset, "isAsset">;
+      delete optIsAsset["isAsset"];
+    }
+  }
+
+  return esm;
+}

__tests__/integration/no-errors.spec.ts

@@ -0,0 +1,91 @@
+import { jest, afterAll, test, expect } from "@jest/globals";
+import * as path from "path";
+import * as fs from "fs-extra";
+
+import { RPT2Options } from "../../src/index";
+import * as helpers from "./helpers";
+
+// increase timeout to 15s for whole file since CI occassionally timed out -- these are integration and cache tests, so longer timeout is warranted
+jest.setTimeout(15000);
+
+const local = (x: string) => path.resolve(__dirname, x);
+const cacheRoot = local("__temp/no-errors/rpt2-cache"); // don't use the one in node_modules
+
+afterAll(() => fs.remove(cacheRoot));
+
+async function genBundle(relInput: string, extraOpts?: RPT2Options) {
+  return helpers.genBundle({
+    input: local(`fixtures/no-errors/${relInput}`),
+    tsconfig: local("fixtures/no-errors/tsconfig.json"),
+    cacheRoot,
+    extraOpts,
+  });
+}
+
+test("integration - no errors", async () => {
+  const { output } = await genBundle("index.ts", { clean: true });
+
+  // populate the cache
+  await genBundle("index.ts");
+  const { output: outputWithCache } = await genBundle("index.ts");
+  expect(output).toEqual(outputWithCache);
+
+  expect(output[0].fileName).toEqual("index.js");
+  expect(output[1].fileName).toEqual("index.d.ts");
+  expect(output[2].fileName).toEqual("index.d.ts.map");
+  expect(output[3].fileName).toEqual("some-import.d.ts");
+  expect(output[4].fileName).toEqual("some-import.d.ts.map");
+  expect(output[5].fileName).toEqual("type-only-import.d.ts");
+  expect(output[6].fileName).toEqual("type-only-import.d.ts.map");
+  expect(output.length).toEqual(7); // no other files
+
+  // JS file should be bundled by Rollup, even though rpt2 does not resolve it (as Rollup natively understands ESM)
+  expect(output[0].code).toEqual(expect.stringContaining("identity"));
+});
+
+test("integration - no errors - no declaration maps", async () => {
+  const noDeclarationMaps = { compilerOptions: { declarationMap: false } };
+  const { output } = await genBundle("index.ts", {
+    tsconfigOverride: noDeclarationMaps,
+    clean: true,
+  });
+
+  expect(output[0].fileName).toEqual("index.js");
+  expect(output[1].fileName).toEqual("index.d.ts");
+  expect(output[2].fileName).toEqual("some-import.d.ts");
+  expect(output[3].fileName).toEqual("type-only-import.d.ts");
+  expect(output.length).toEqual(4); // no other files
+});
+
+
+test("integration - no errors - no declarations", async () => {
+  const noDeclarations = { compilerOptions: { declaration: false, declarationMap: false } };
+  const { output } = await genBundle("index.ts", {
+    tsconfigOverride: noDeclarations,
+    clean: true,
+  });
+
+  expect(output[0].fileName).toEqual("index.js");
+  expect(output.length).toEqual(1); // no other files
+});
+
+test("integration - no errors - allowJs + emitDeclarationOnly", async () => {
+  const { output } = await genBundle("some-js-import.js", {
+    include: ["**/*.js"],
+    tsconfigOverride: {
+      compilerOptions: {
+        allowJs: true,
+        emitDeclarationOnly: true,
+      },
+    },
+  });
+
+  expect(output[0].fileName).toEqual("index.js");
+  expect(output[1].fileName).toEqual("some-js-import.d.ts");
+  expect(output[2].fileName).toEqual("some-js-import.d.ts.map");
+  expect(output.length).toEqual(3); // no other files
+
+  expect(output[0].code).toEqual(expect.stringContaining("identity"));
+  expect(output[0].code).not.toEqual(expect.stringContaining("sum")); // no TS files included
+  expect("source" in output[1] && output[1].source).toEqual(expect.stringContaining("identity"));
+});

jest.config.js

@@ -1,6 +1,14 @@
-/** @type {import("@jest/types").Config.InitialOptions} */
+/** @type {import("ts-jest").InitialOptionsTsJest} */
 const config = {
+	// ts-jest settings
 	preset: "ts-jest",
+	globals: {
+		"ts-jest": {
+			tsconfig: "./tsconfig.test.json",
+		}
+	},
+
+	// jest settings
 	injectGlobals: false, // use @jest/globals instead
 	restoreMocks: true,
 	// only use *.spec.ts files in __tests__, no auto-generated files

tsconfig.test.json

@@ -0,0 +1,6 @@
+{
+	"extends": "./tsconfig",
+	"compilerOptions": {
+		"esModuleInterop": true, // needed to parse some imports with ts-jest (since Rollup isn't used when importing during testing)
+	},
+}