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

Tests failing #101

Closed
zachasme opened this issue Sep 4, 2019 · 13 comments · Fixed by #103
Closed

Tests failing #101

zachasme opened this issue Sep 4, 2019 · 13 comments · Fixed by #103

Comments

@zachasme
Copy link
Contributor

zachasme commented Sep 4, 2019

This library is generating types for Ubers h3-js bindings which I depend on. But the generated types for generics are looking strange.

  • T[] becomes [ 'Array' ]<T>
  • Array<T> becomes [ 'Array' ]<T>
  • Promise<T> becomes [ 'Promise' ]<T>

Tests in this repo are failing on a fresh clone as well.

I've tried figuring out if it was a recent commit or a backwards-incompatible dependency update but I'm unable to figure out the cause. If I could be pointed in the right direction I'll try and submit a PR.

@englercj
Copy link
Owner

englercj commented Sep 4, 2019

Haven't taken a close look but local tests are failing for me too, though CI passed for the latest commit which is strange.

@aidinabedi are you seeing the failures as well? Does this seem related to your recent changes?

@aidinabedi
Copy link
Contributor

Haven't seen any of these behaviors. In fact, we use both Array<T> and T[] successfully in our project. @zachasme Could you provide a minimal test case that reproduces the issue?

@englercj
Copy link
Owner

englercj commented Sep 4, 2019

Running npm test fails locally for me. Can you try running npm i to make sure you match the latest lock and run tests again?

@aidinabedi
Copy link
Contributor

aidinabedi commented Sep 5, 2019

Super weird. I just made two fresh clones (one of this repo and other of my fork), both matches the latest lock and both passes all tests.

What test fails for you, @englercj?
What platform are you running on?

My platform:
Windows 10
Node 8.12.0
NPM 6.4.1

@zachasme
Copy link
Contributor Author

zachasme commented Sep 5, 2019

Installing using npm i with the latest lock results in tests throwing ENOENT: no such file or directory, open '/home/zach/dev/forks/tsd-jsdoc/test/_temp/types.d.ts' for every test (even after manually running npm run build).

Installing using yarn or npm i after deleting package-lock.json results in the [ 'Array' ]<T> typings.

Platform: Arch Linux, Node v12.10.0, NPM v6.11.3

@aidinabedi
Copy link
Contributor

aidinabedi commented Sep 5, 2019

I can't seem to reproduce the issue you describe...

PS E:\Temp\tsd-jsdoc> git clone https://github.com/englercj/tsd-jsdoc.git .

Cloning into '.'...
remote: Enumerating objects: 55, done.
remote: Counting objects: 100% (55/55), done.
remote: Compressing objects: 100% (51/51), done.
remote: Total 861 (delta 24), reused 16 (delta 4), pack-reused 806
Receiving objects: 100% (861/861), 347.19 KiB | 0 bytes/s, done.
Resolving deltas: 100% (504/504), done.

PS E:\Temp\tsd-jsdoc> del package-lock.json

PS E:\Temp\tsd-jsdoc> npm i

> tsd-jsdoc@2.4.0 prepare E:\Temp\tsd-jsdoc
> npm run build


> tsd-jsdoc@2.4.0 build E:\Temp\tsd-jsdoc
> tsc -p tsconfig.json

npm notice created a lockfile as package-lock.json. You should commit this file.
added 90 packages from 570 contributors and audited 134 packages in 7.026s
found 1 moderate severity vulnerability
  run `npm audit fix` to fix them, or `npm audit` for details

PS E:\Temp\tsd-jsdoc> npm test

> tsd-jsdoc@2.4.0 test E:\Temp\tsd-jsdoc
> mocha --ui tdd -r ts-node/register test/specs/**.ts



  Class Checks
    √ All (641ms)

  Enum Checks
    √ All (538ms)

  Function Checks
    √ All (547ms)

  Interface Checks
    √ All (559ms)

  Module Checks
    √ All (556ms)

  Namespace Checks
    √ All (586ms)

  Property Checks
    √ All (558ms)

  Typedef Checks
    √ All (568ms)


  8 passing (5s)

@zachasme
Copy link
Contributor Author

zachasme commented Sep 5, 2019

I'm following the exact same steps. I tried it on Windows as well - same problem.

However I also tried downgrading node to v10.16.3 after which all the tests pass!

@aidinabedi
Copy link
Contributor

aidinabedi commented Sep 5, 2019

Just ran everything at work (node 10.11.0) and all the tests pass.

Likely the behavior of some node API that has changed between v10-12.

@englercj
Copy link
Owner

englercj commented Sep 6, 2019

I am using node v12, so seems like it is related to the node version. I wonder if this issue is on the jsdoc side or ours.

@aidinabedi
Copy link
Contributor

Was thinking the same thing. Seems most plausible.

@zachasme
Copy link
Contributor Author

zachasme commented Sep 11, 2019

Directly printing the data that jsdoc parsed using (in publish.ts)

const docs = data().get();
const out = path.join(opts.destination, `debug.js`);
fs.writeFileSync(out, JSON.stringify(docs));

already contains the problematic typename

{
    type: { names: ["[ 'Array' ].<T>"] },
    description: "The third param.",
    name: "third"
}

so I'll wager the issue is on the jsdoc side.

EDIT: Running a similar source through jsdoc directly (using the same template) emits correct typenames, so jsdoc-api is possibly to blame.

@zachasme
Copy link
Contributor Author

It seems to have been fixed in jsdoc-api@latest. However there are changes to at least @template handling. Would you accept a PR upgrading dependencies and fixing tests?

@englercj
Copy link
Owner

Absolutely.

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

Successfully merging a pull request may close this issue.

3 participants