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.

Sign up for GitHub

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

[New] add types #603

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+590 −49
Open

[New] add types #603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions .github/workflows/node-aught.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ jobs:
range: '< 10'
type: minors
command: npm run tests-only
skip-ls-check: true
1 change: 1 addition & 0 deletions bin/import-or-require.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { extname: extnamePath } = require('path');
const { pathToFileURL } = require('url');
const getPackageType = require('get-package-type');

/** @type {(file: string) => undefined | Promise<unknown>} */
ljharb marked this conversation as resolved.
Show resolved Hide resolved
// eslint-disable-next-line consistent-return
module.exports = function importOrRequire(file) {
const ext = extnamePath(file);
Expand Down
2 changes: 2 additions & 0 deletions bin/tape
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,11 @@ var hasImport = require('has-dynamic-import');

var tape = require('../');

/** @type {(hasSupport: boolean) => Promise<void> | void} */
function importFiles(hasSupport) {
tape.wait();

/** @type {null | undefined | Promise<unknown>} */
var filesPromise;
if (hasSupport) {
var importOrRequire = require('./import-or-require');
Expand Down
76 changes: 76 additions & 0 deletions index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import type { ThroughStream } from '@ljharb/through';

import type Test from './lib/test';
import type Results from './lib/results';

declare function harnessFunction(this: Test, name: string, opts: tape.TestOptions, cb: Test.Callback): Test;
declare function harnessFunction(this: Test, name: string, opts: tape.TestOptions): Test;
declare function harnessFunction(this: Test, name: string, cb: Test.Callback): Test;
declare function harnessFunction(this: Test, name: string): Test;
declare function harnessFunction(this: Test, opts: tape.TestOptions, cb: Test.Callback): Test;
declare function harnessFunction(this: Test, opts: tape.TestOptions): Test;
declare function harnessFunction(this: Test, cb: Test.Callback): Test;

declare namespace tape {
export type TestOptions = {
objectPrintDepth?: number | undefined;
skip?: boolean | undefined;
timeout?: number | undefined;
todo?: boolean | undefined;
};

export interface AssertOptions {
skip?: boolean | string | undefined;
todo?: boolean | string | undefined;
message?: string | undefined;
actual?: unknown;
expected?: unknown;
exiting?: boolean;
}

export interface StreamOptions {
objectMode?: boolean | undefined;
}

function createStream(opts?: StreamOptions): ThroughStream;

export type CreateStream = typeof createStream;

export type HarnessEventHandler = (cb: Test.SyncCallback, ...rest: unknown[]) => void;

function only(name: string, cb: Test.Callback): void;
function only(name: string, opts: tape.TestOptions, cb: Test.Callback): void;
function only(cb: Test.Callback): void;
function only(opts: tape.TestOptions, cb: Test.Callback): void;

export type Harness = typeof harnessFunction & {
run?: () => void;
only: typeof only;
_exitCode: number;
_results: Results;
_tests: Test[];
close: () => void;
createStream: CreateStream;
onFailure: HarnessEventHandler;
onFinish: HarnessEventHandler;
}

export type HarnessConfig = {
autoclose?: boolean;
noOnly?: boolean;
stream?: NodeJS.WritableStream | ThroughStream;
exit?: boolean;
} & StreamOptions;

function createHarness(conf_?: HarnessConfig): Harness;
const Test: Test;
const test: typeof tape;
const skip: Test['skip'];
ljharb marked this conversation as resolved.
Show resolved Hide resolved
}

declare function tape(this: tape.Harness, name: string, opts: tape.TestOptions, cb: Test.Callback): Test;
declare function tape(this: tape.Harness, name: string, cb: Test.Callback): Test;
declare function tape(this: tape.Harness, opts?: tape.TestOptions): Test;
declare function tape(this: tape.Harness, opts: tape.TestOptions, cb: Test.Callback): Test;

export = tape;
48 changes: 38 additions & 10 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,34 @@
'use strict';

var defined = require('defined');
var through = require('@ljharb/through');

var createDefaultStream = require('./lib/default_stream');
var Test = require('./lib/test');
var Results = require('./lib/results');
var through = require('@ljharb/through');

var canEmitExit = typeof process !== 'undefined' && process
&& typeof process.on === 'function' && process.browser !== true;
// eslint-disable-next-line no-extra-parens
&& typeof process.on === 'function' && /** @type {{ browser?: boolean }} */ (process).browser !== true;
var canExit = typeof process !== 'undefined' && process
&& typeof process.exit === 'function';

module.exports = (function () {
/** @typedef {import('.')} Tape */
/** @typedef {import('.').Harness} Harness */
/** @typedef {import('.').HarnessConfig} HarnessConfig */
/** @typedef {import('.').TestOptions} TestOptions */
/** @typedef {import('.').HarnessEventHandler} HarnessEventHandler */
/** @typedef {import('.').CreateStream} CreateStream */
/** @typedef {import('.').createHarness} CreateHarness */
/** @typedef {import('./lib/results').Result} Result */
/** @typedef {import('stream').Writable} WritableStream */

var tape = (function () {
var wait = false;
/** @type {undefined | Harness} */
var harness;

/** @type {(opts?: HarnessConfig) => Harness} */
function getHarness(opts) {
// this override is here since tests fail via nyc if createHarness is moved upwards
if (!harness) {
Expand All @@ -24,6 +38,7 @@ module.exports = (function () {
return harness;
}

/** @type {(this: Harness, ...args: Parameters<Tape>) => ReturnType<Tape>} */
function lazyLoad() {
// eslint-disable-next-line no-invalid-this
return getHarness().apply(this, arguments);
Expand All @@ -43,6 +58,7 @@ module.exports = (function () {
return getHarness().only.apply(this, arguments);
};

/** @type {CreateStream} */
lazyLoad.createStream = function (opts) {
var options = opts || {};
if (!harness) {
Expand All @@ -66,21 +82,23 @@ module.exports = (function () {
return lazyLoad;
}());

/** @type {CreateHarness} */
function createHarness(conf_) {
var results = new Results({ todoIsOK: !!(process.env.TODO_IS_OK === '1') });
if (!conf_ || conf_.autoclose !== false) {
results.once('done', function () { results.close(); });
}

/** @type {(name: string, conf: TestOptions, cb: Test.Callback) => Test} */
function test(name, conf, cb) {
var t = new Test(name, conf, cb);
test._tests.push(t);

(function inspectCode(st) {
st.on('test', function sub(st_) {
st.on('test', /** @type {(st: Test) => void} */ function sub(st_) {
inspectCode(st_);
});
st.on('result', function (r) {
st.on('result', /** @type {(r: Result) => void} */ function (r) {
if (!r.todo && !r.ok && typeof r !== 'string') { test._exitCode = 1; }
});
}(t));
Expand All @@ -90,21 +108,25 @@ function createHarness(conf_) {
}
test._results = results;

test._tests = [];
/** @type {Test[]} */ test._tests = [];

/** @type {CreateStream} */
test.createStream = function (opts) {
return results.createStream(opts);
};

/** @type {HarnessEventHandler} */
test.onFinish = function (cb) {
results.on('done', cb);
};

/** @type {HarnessEventHandler} */
test.onFailure = function (cb) {
results.on('fail', cb);
};

var only = false;
/** @type {() => Test} */
test.only = function () {
if (only) { throw new Error('there can only be one only test'); }
if (conf_ && conf_.noOnly) { throw new Error('`only` tests are prohibited'); }
Expand All @@ -117,9 +139,11 @@ function createHarness(conf_) {

test.close = function () { results.close(); };

// @ts-expect-error TODO FIXME: why is `test` not assignable to `Harness`???
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

The type error occurs because the test function is missing required properties from Harness type

Looking at the type definitions, Harness is defined as typeof harnessFunction & {...}, which means it must have both the function signature and additional properties. The error occurs because while the test function has some properties added (_exitCode, close), it's missing other required properties from the Harness type like run, only, _results, and _tests.

To fix this:

  • Add all required properties from the Harness type to the test function
  • Or modify the type assertion to handle partial implementation
🔗 Analysis chain

Address the TODO comment regarding type compatibility.

The comment indicates that test is not assignable to Harness. This might indicate a structural incompatibility between the types that should be investigated.

Would you like help investigating and resolving the type compatibility issue?

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the definition of Harness type and its usage
ast-grep --pattern $'interface Harness {
  $$$
}'

Length of output: 50

Script:

#!/bin/bash
# Let's try different approaches to find Harness and test related code

# Search for Harness type/interface/class definition
rg -A 5 "(?:type|interface|class)\s+Harness"

# Search for the specific TODO comment and its context
rg -B 5 -A 5 "TODO FIXME: why is .test. not assignable to .Harness"

# Search for test variable declaration and usage
ast-grep --pattern "test = $_"

Length of output: 1518

Script:

#!/bin/bash
# Let's examine the test function definition and its properties
rg -B 10 -A 5 "function createHarness"

# Look for the harnessFunction type definition
rg -A 5 "harnessFunction"

# Check the test variable's type annotations
ast-grep --pattern "/** @type {$_} */"

Length of output: 2347

return test;
}

/** @type {(conf: Omit<HarnessConfig, 'autoclose'>, wait?: boolean) => Harness} */
ljharb marked this conversation as resolved.
Show resolved Hide resolved
ljharb marked this conversation as resolved.
Show resolved Hide resolved
function createExitHarness(config, wait) {
var noOnly = config.noOnly;
var objectMode = config.objectMode;
Expand All @@ -137,11 +161,12 @@ function createExitHarness(config, wait) {
if (running) { return; }
running = true;
var stream = harness.createStream({ objectMode: objectMode });
var es = stream.pipe(cStream || createDefaultStream());
// eslint-disable-next-line no-extra-parens
var es = stream.pipe(/** @type {WritableStream} */ (cStream || createDefaultStream()));
if (canEmitExit && es) { // in node v0.4, `es` is `undefined`
// TODO: use `err` arg?
// eslint-disable-next-line no-unused-vars
es.on('error', function (err) { harness._exitCode = 1; });
es.on('error', function (_) { harness._exitCode = 1; });
}
stream.on('end', function () { ended = true; });
}
Expand Down Expand Up @@ -179,7 +204,10 @@ function createExitHarness(config, wait) {
return harness;
}

module.exports = tape;

module.exports.createHarness = createHarness;
// var moduleExports = module.exports; // this hack is needed because TS has a bug with seemingly circular exports
module.exports.Test = Test;
module.exports.test = module.exports; // tap compat
module.exports.test.skip = Test.skip;
module.exports.test = tape; // tap compat
module.exports.skip = Test.skip;
5 changes: 5 additions & 0 deletions lib/default_stream.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import type { ThroughStream } from "@ljharb/through";

declare function defaultStream(): ThroughStream;

export = defaultStream;
11 changes: 7 additions & 4 deletions lib/default_stream.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
var through = require('@ljharb/through');
var fs = require('fs');

/** @type {import('./default_stream')} */
module.exports = function () {
var line = '';
var stream = through(write, flush);
return stream;

/** @type {(buf: unknown) => void} */
function write(buf) {
if (
buf == null // eslint-disable-line eqeqeq
ljharb marked this conversation as resolved.
Show resolved Hide resolved
ljharb marked this conversation as resolved.
Show resolved Hide resolved
ljharb marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -16,10 +18,11 @@ module.exports = function () {
flush();
return;
}
for (var i = 0; i < buf.length; i++) {
var c = typeof buf === 'string'
? buf.charAt(i)
: String.fromCharCode(buf[i]);
var b = /** @type {string | ArrayLike<number>} */ (buf); // eslint-disable-line no-extra-parens
ljharb marked this conversation as resolved.
Show resolved Hide resolved
for (var i = 0; i < b.length; i++) {
var c = typeof b === 'string'
? b.charAt(i)
: String.fromCharCode(b[i]);
ljharb marked this conversation as resolved.
Show resolved Hide resolved
ljharb marked this conversation as resolved.
Show resolved Hide resolved
if (c === '\n') {
flush();
} else {
Comment on lines 18 to 28
Copy link

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [12-25]

The casting of buf to string | ArrayLike<number> is sensible given the function's need to support both string and buffer inputs. However, ensure that this casting is safe by validating the input type before casting, as incorrect assumptions about the type can lead to runtime errors.

Consider adding explicit type checks before casting to ensure safety.

Expand Down
52 changes: 52 additions & 0 deletions lib/results.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import through from '@ljharb/through';
import type { EventEmitter } from 'events';

import type { StreamOptions } from '../';
import Test = require('./test');

declare class Results extends EventEmitter {
constructor(options?: { todoIsOK?: boolean });

count: number;
fail: number;
pass: number;
tests: Test[];
todo: number;
todoIsOK: boolean;
closed?: boolean;

_isRunning: boolean;
_only: Test | null;
_stream: through.ThroughStream;
ljharb marked this conversation as resolved.
Show resolved Hide resolved

close(this: Results): void;
createStream(this: Results, opts?: StreamOptions): through.ThroughStream;
only(this: Results, t: Test): void;
push(this: Results, t: Test): void;

_watch(this: Results, t: Test): void;
ljharb marked this conversation as resolved.
Show resolved Hide resolved
}

declare namespace Results {
export type Operator = string;

export type Result = {
id: number;
ok: boolean;
skip: unknown;
todo: unknown;
name?: string;
operator: undefined | Operator;
objectPrintDepth?: number;
actual?: unknown;
expected?: unknown;
error?: unknown;
functionName?: string;
file?: string;
line?: number;
column?: number;
at?: string;
};
}

export = Results;
Loading
Loading