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

aegir test --ts -t browser seems to fail #619

Closed
Gozala opened this issue Aug 7, 2020 · 8 comments · Fixed by #737
Closed

aegir test --ts -t browser seems to fail #619

Gozala opened this issue Aug 7, 2020 · 8 comments · Fixed by #737

Comments

@Gozala
Copy link
Contributor

Gozala commented Aug 7, 2020

It appears that aegir test --ts -t browser fails with timeout where ageir test -t browser does not:

multiformats/js-multiaddr@5a4b2df

Gozala added a commit to multiformats/js-multiaddr that referenced this issue Aug 7, 2020
jacobheun pushed a commit to multiformats/js-multiaddr that referenced this issue Aug 10, 2020
* fix: disable linter for types/types.spec.ts

* fix: use custom eslintrc instead

* fix: lint errors

* chore: enable ts support for tests

* fix: workaround ipfs/aegir#619
jacobheun pushed a commit to multiformats/js-multiaddr that referenced this issue Aug 10, 2020
* chore(deps-dev): bump aegir from 22.1.0 to 25.0.0

Bumps [aegir](https://github.com/ipfs/aegir) from 22.1.0 to 25.0.0.
- [Release notes](https://github.com/ipfs/aegir/releases)
- [Changelog](https://github.com/ipfs/aegir/blob/master/CHANGELOG.md)
- [Commits](ipfs/aegir@v22.1.0...v25.0.0)

Signed-off-by: dependabot-preview[bot] <[email protected]>

* fix: use custom .eslintrc with different rules for .js and .ts (#141)

* fix: disable linter for types/types.spec.ts

* fix: use custom eslintrc instead

* fix: lint errors

* chore: enable ts support for tests

* fix: workaround ipfs/aegir#619

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Irakli Gozalishvili <[email protected]>
@jacobheun
Copy link
Contributor

This also creates some blocking issues for running tests during release. ref multiformats/js-multiaddr#154

@hugomrdias
Copy link
Member

why are you using --ts if multiaddr is not written in ts ?

@Gozala
Copy link
Contributor Author

Gozala commented Nov 19, 2020

@hugomrdias here is the thread with the whole story
multiformats/js-multiaddr#131 (comment)

@hugomrdias
Copy link
Member

the problem is mixing commonjs with ts, we only support full ts or full commonjs with jsdoc.
plus this file https://github.com/multiformats/js-multiaddr/blob/master/test/types.spec.ts is testing nothing

@hugomrdias
Copy link
Member

testing types is done with aegir ts -p check which will be released soon

@Gozala
Copy link
Contributor Author

Gozala commented Nov 23, 2020

Not sure I follow all this, but that file was added by multiformats/js-multiaddr#112 and from the pull it appears it was introduced to ensure exported types did type-check.

Maybe with aegir ts -p check we no longer need it, but I am not entirely sure. Even if things type check and we generate some types that does not mean that those types would come out the way we expect. That said maybe it is reasonable to assume that and not bother having tests for it.

@hugomrdias
Copy link
Member

theres .d.ts tests (we can add support for that), but that is a separate thing.
we can't handle mixed projects with the current setup.
i will handle this in that repo and aegir itself.

@hugomrdias
Copy link
Member

hugomrdias commented Nov 24, 2020

what we need in aegir is to not include .ts files in aegir test if --ts is not true

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