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

Migrate test cases into typescript #1364

Merged
merged 8 commits into from
Mar 1, 2016

Conversation

kwonoj
Copy link
Member

@kwonoj kwonoj commented Feb 19, 2016

closes #661.

This PR aims to use existing unit test environment for type definition validation as well by migrating into typescript based. As in first step of incremental changes, this specific PR targets migrating test cases only without adding any other changes such as adding type validation tests, fix type definition in code bases, etcs.

To avoid test cases are breaking while migration this PR includes huge chunk of file changes, but most of them are simply amending existing test cases into typescript friendly. High level changes are as below.

  • all test spec, helpers are now typescript along with migrated other test chains such as coverage, doctest
  • introduced npm script test_buildonly : this will compile test only without executing it for cases like type validation test running.
  • introduced npm script test_nobuild: in opposite to above command, this'll execute existing compiled test without rebuild, reduces test turnaround time when test cases are infrequently changes
  • existing script test will execute test_buildonly && test_nobuild

Further goals

  • add actual test cases for type validation, fix type definition as well
  • refactor test helper, support test framework independent / modular / better injection (no global patches, etcs)
  • migrate few missing tool chains (test2png)

Opinion, suggestions are gladly welcome.

/cc @david-driscoll , @robwormald for visibility.

@kwonoj kwonoj force-pushed the jasmine-typescript branch 3 times, most recently from 7004c10 to b101fc4 Compare February 20, 2016 00:56
@staltz
Copy link
Member

staltz commented Feb 22, 2016

wow! 👏

@robwormald
Copy link
Contributor

Sorry, we could not display the entire diff because it was too big. is when you know somebody is putting in serious work 👍

@david-driscoll
Copy link
Member

@kwonoj holy wow! 👏 🎩

I like that code coverage increased too. 👍

@benlesh
Copy link
Member

benlesh commented Mar 1, 2016

This is nuts. In a good way. I'm super pleased with this PR. Looks good to me so far.

Question: When you were doing this, did you git mv the files so it realizes you just renamed them?

@kwonoj
Copy link
Member Author

kwonoj commented Mar 1, 2016

When you were doing this, did you git mv the files so it realizes you just renamed them?

: actually I didn't, some of files were auto recognized by git as renamed and some weren't. :( Shall I apply it and update PR accordingly?

@benlesh
Copy link
Member

benlesh commented Mar 1, 2016

@kwonoj: Looks good to me other than some of the other npm scripts now seem "dead". build_test and build_cover don't seem to find any specs. Those are the two I was entering most often. Perhaps remove them for now or just have them echo a message to try the new scripts?

@kwonoj
Copy link
Member Author

kwonoj commented Mar 1, 2016

build_test and build_cover don't seem to find any specs

: I've completely missed those, my bad. Instead of removing it, amended those script to include test case build also. I'll think about fine grained script control in further. (such as combination of build src / test, + run test + coverage...)

@benlesh
Copy link
Member

benlesh commented Mar 1, 2016

Okay... LGTM

@kwonoj kwonoj merged commit 4bc32b7 into ReactiveX:master Mar 1, 2016
@kwonoj
Copy link
Member Author

kwonoj commented Mar 1, 2016

Merged with 4bc32b7. Now one step closer to more type-friendly codebase :)

I'll work on followup changes to update type definitions as well.

@kwonoj kwonoj deleted the jasmine-typescript branch March 1, 2016 17:53
@lock
Copy link

lock bot commented Jun 7, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add test for type definition validation?
5 participants