Skip to content

Add unit tests#135

Closed
brekk wants to merge 4 commits intoezolenko:masterfrom
brekk:add-unit-tests
Closed

Add unit tests#135
brekk wants to merge 4 commits intoezolenko:masterfrom
brekk:add-unit-tests

Conversation

@brekk
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Owner

ezolenko commented Feb 5, 2019

Thanks! I'll check it out soon

@brekk
Copy link
Copy Markdown
Contributor Author

brekk commented Apr 19, 2019

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

@ezolenko
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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