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

Proposal : migrate unit test to typescript #826

Closed
wants to merge 1 commit into from

Conversation

kwonoj
Copy link
Member

@kwonoj kwonoj commented Nov 29, 2015

Not to be merged since it's for proposal

Related with #661, #667

One of other possibilities of enhancing type definition validation is using unit test instead of having additional test for definition only, as same as other (such as angular 2) does. This PR proposes basic setups of migrating existing test cases into typescript based, to gather opinions to proceed or not.

This PR does

  • reuses most of existing test fixture setup (marble-testing helper function, test-helper) as is, with small modifications. Additional refactoring for typescript specific can be considered further once if test case is actually migrated.
  • picks one existing case (count-spec) convert it into typescript based. Major difference is importing helper function (hot, cold, ...) explicitly instead of using global.
  • for rxjs build, test case still uses cjs output instead of directly referencing typescript source to reduce build time for test.

Travis currently fails by markdown-doctest is not updated with this changes.

@kwonoj kwonoj added the blocked label Nov 29, 2015
@benlesh
Copy link
Member

benlesh commented Nov 29, 2015

I'm on my phone, so I can't fully review this, but it's probably worth building all jasmine test helpers into their own module. At some point we'll want to publish that for others to use anyhow.

@kwonoj
Copy link
Member Author

kwonoj commented Nov 29, 2015

it's probably worth building all jasmine test helpers into their own module.

: Yes, good point.

I'll leave this specific PR without updating it to discuss go / no-go first, once it's decided to proceed will update PR accordingly.

@staltz
Copy link
Member

staltz commented Nov 30, 2015

Travis currently fails by markdown-doctest is not updated with this changes.

By the way, I think we should make markdown-doctest errors be just warnings. Either that, or we need to find a way of explicitly ignoring some parts. @Widdershin ?

@kwonoj
Copy link
Member Author

kwonoj commented Nov 30, 2015

I think we should make markdown-doctest errors be just warnings.

: Isn't purpose of markdown-doctest is let it fail if it 'fails'? ;)

@Widdershin
Copy link
Contributor

@staltz If a specific example shouldn't be tested, you can prefix it with <!-- skip-example -->.

@Widdershin
Copy link
Contributor

I'm also happy to add support for Typescript to markdown-doctest.

@staltz
Copy link
Member

staltz commented Nov 30, 2015

If a specific example shouldn't be tested, you can prefix it with <!-- skip-example -->.

Ok, that's good, I might use it. My case wasn't typescript code, it was Jasmine code, it didn't recognize it('should be') type of specs.

@Widdershin
Copy link
Contributor

@staltz yell out if you want any help integrating markdown-doctest into a project, I've been meaning to set it up for some of the cycle repos.

@kwonoj
Copy link
Member Author

kwonoj commented Nov 30, 2015

I'm also happy to add support for Typescript to markdown-doctest

: Appreciate for it, let's discuss in further depends on decisions made :)

@staltz
Copy link
Member

staltz commented Nov 30, 2015

@Widdershin let's discuss in #757, so this PR discussion stays on topic.

@benlesh
Copy link
Member

benlesh commented Jan 25, 2016

@kwonoj Discussion is dead here. While I'd like to move our tests to TypeScript for a variety of reasons, I don't see this particular PR going anywhere. Closing for now, feel free to reopen if you disagree, I'm just cleaning up.

@benlesh benlesh closed this Jan 25, 2016
@kwonoj
Copy link
Member Author

kwonoj commented Jan 25, 2016

I think it's ok to close, issue #661 can be used to get conclusion where to start. I own responsibility to wrap up those discussion to which way to go.

@kwonoj kwonoj deleted the test-typescript branch March 8, 2016 20:44
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants