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

Add unit tests #135

Closed
wants to merge 4 commits into from
Closed

Add unit tests #135

wants to merge 4 commits into from

Conversation

brekk
Copy link
Contributor

@brekk brekk commented Feb 2, 2019

Hey @ezolenko,

I saw in the README that you needed some unit tests -- I tried to add some decent coverage for the files here (though I think I may have misunderstood the build process (specifically the tsProxy stuff); if this branch is useful I'm happy to amend / continue this branch but as I'm about to start a new job in a couple days I figured it would be better to post this as is rather than let it languish.

screen shot 2019-02-02 at 1 58 16 pm

Additionally this is the first time I've written unit tests for TS -- I did some dirty checking of instances via instance as any but I think in general it has decent coverage for the files I started tests for.

I hope it's ok, feedback appreciated.

Thanks,

Brekk

@brekk
Copy link
Contributor Author

brekk commented Feb 2, 2019

Oh, also yarn test or npm run test or jest --verbose --coverage are the current ways I run tests.

@ezolenko
Copy link
Owner

ezolenko commented Feb 5, 2019

Thanks! I'll check it out soon

@brekk
Copy link
Contributor Author

brekk commented Apr 19, 2019

@ezolenko Hey I was just curious if there was any traction on this.

@ezolenko
Copy link
Owner

Yes and no sorry, I have problems executing this on windows, but I haven't looked too closely on what exactly was wrong yet.

@brekk
Copy link
Contributor Author

brekk commented Apr 23, 2019

@ezolenko I tried running the tests on my windows machine at work and was able to uncover two failures in the test suite which I have recently addressed -- please let me know if it ends up working on your end. Actually I was running in the ubuntu shell on my windows machine -- I will do a bit more to test this out when I have some time.

@ezolenko
Copy link
Owner

Thanks for the updates.

Could you merge everything from master and run a npm build && npm build-self && npm build-self cycle? I'm getting strange things from rollup when it makes bundle due to the way the imports are changed I think.

Also, could you move *.spec.ts files into their own directory at the same level as source, so there is <root>/src/*.ts and <root>/unittests/*.spec.ts or something and make sure unittest artifacts don't make it into dest. (I can do that part myself once things are working if you don't get around to it)

@agilgur5
Copy link
Collaborator

I'm hoping to pick this PR/branch up soon, update it, and fix any issues. This plugin could really use some tests and this PR has been on my radar for a while.

Will of course credit OP and use their commits as-is where possible (pending merge issues) or otherwise make them co-author if I make a new PR that gets squashed in.

@agilgur5
Copy link
Collaborator

agilgur5 commented May 3, 2022

Have a WIP branch of this now, fully rebased with 3 years of commits, updated deps, updated config, moved to __tests__ dir, etc.
Working on updating the tests, fixing any build issues, and then adding CI action now; PR coming soon!

@agilgur5 agilgur5 added priority: in progress scope: tests Tests could be improved. Or changes that only affect tests kind: feature New feature or request problem: stale Issue has not been responded to in some time labels May 3, 2022
@agilgur5
Copy link
Collaborator

agilgur5 commented May 8, 2022

Replaced by #321 . Thanks again for writing this initial batch!

@agilgur5 agilgur5 closed this May 8, 2022
@agilgur5 agilgur5 added kind: internal Changes only affect the internals, and _not_ the public API or external-facing docs and removed kind: feature New feature or request labels May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: internal Changes only affect the internals, and _not_ the public API or external-facing docs problem: stale Issue has not been responded to in some time scope: tests Tests could be improved. Or changes that only affect tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants