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

Source test #82

Open
unional opened this issue Jun 2, 2016 · 11 comments
Open

Source test #82

unional opened this issue Jun 2, 2016 · 11 comments

Comments

@unional
Copy link
Collaborator

unional commented Jun 2, 2016

Current source test is a manual process.

Would be great to create source test for some basic scenarios.

Specific scenarios will be determined and spilted into different issues in the future.

@felixfbecker
Copy link

@unional I don't get this whole "source-test" generated by the generator. Typings contain only types. Verifying that they work means only to set up some usual TypeScript example code (preferable from the docs) and check if it compiles without error. What exactly is a source test? Why does a generated typings repo include a dev dependency on blue-tap? Why can you get into cases where it is downloading the whole of electron?

@unional
Copy link
Collaborator Author

unional commented Sep 12, 2016

Please see https://github.com/typings/generator-typings#about-writing-tests-for-typings for why shape test (just compile without error) is not sufficient.

source test is a folder that for reusing tests from the source by copying and modifying the tests from source minimally and run them under TypeScript.

It may not be needed if this is implemented: microsoft/TypeScript#7661

@felixfbecker
Copy link

Simply shape test (like those in DefinitlyType) is not sufficient.
Since there is no type in javascript, even if you create a wrong signature, you won't detect it until runtime.

e.g.

// source code
function foo(something) {
  return something++;
}

The source code expects something to be a number. If you write your typings as:

function foo(something: string): string;

It won't fail until you use it. i.e.

// consumer program
import foo ....somehow

let x = "abc";
foo(x);

Because your typings provide guidance to the consumer, they will write their code that way and will fail when they run it.
tsc will compile fine.

That is not true. If you have foo defined accepting a string as in the example above, and then add tests (from the source test suite or from docs examples) that call foo with a number, tsc will not compile fine.

@unional
Copy link
Collaborator Author

unional commented Sep 12, 2016

Yes, but you can pass in a string and it will compile fine, which is invalid to begin with.

It should be written as function foo(something: number) {

@unional
Copy link
Collaborator Author

unional commented Sep 12, 2016

This is a major flaw in the DT system today. You can't really run the tests, only compile them.

@felixfbecker
Copy link

But that is not the point here. The unit under test here are the type declarations. To verify that they are correct, we have to compile correct TypeScript code (from official examples) with the declarations, and if they compile, the type declarations are correct. Type declarations have nothing to do with runtime, they only give compile-time checks. Running the actual tests is of no use, testing the runtime behavior of the library is not the task of the declaration repo, but of the library tests.

@felixfbecker
Copy link

felixfbecker commented Sep 12, 2016

Let's take a step back.

Goal

Want to verify the correctness of type declarations.

Approach

Compile correct code takenfrom examples or source test.

  • if it compiles, the declarations are correct
  • if it doesn't, the declarations are incorrect

Example

Test

assumed to be correct because taken from docs or source test

const x: number = foo(1234);

Correct typings

function foo(something: number): number;

✔️ will compile fine

Incorrect typings

let's imagine a PR changed this for whatever reason, we can all imagine more complicated examples

function foo(something: string): number;

❌ tsc will fail with an error


Coming from this, where do we need "source-test" that are actually run and not just compiled? What additional benefit do they provide?

@blakeembrey
Copy link
Collaborator

blakeembrey commented Sep 12, 2016

@felixfbecker I think the thing about testing the runtime here is important. It ensures that we have typed the definition correctly. Otherwise we're just eyeballing it all the way and making educated guesses - we haven't codified that this is actually how things run. This is even more important here when the original source was untyped. For instance, testing the runtime would fix a lot of people using the wrong type of export or make sure the documentation they read is actually correct or that real-world usage is possible. Also, signatures may change between versions and you may miss documentation changes.

With that said, source-test is very confusing and I'm yet to see it used. I think it's probably to painful of an investment and something that could be moved away from being included by default into something that's a "hey, here's an idea for testing the definition you just wrote - check out the libraries tests on GitHub".

Edit: In summary, by only doing compilation as a test, all your doing is testing yourself. Your test could be wrong from the beginning because you made incorrect assumptions.

@unional
Copy link
Collaborator Author

unional commented Sep 12, 2016

I agree that it is confusing and hard to use. I had a hard time to automate it to begin with.
The rationale of it is to reuse tests from source so that we don't have to write tests that already existed.

In 2.0 I am making it optional and also add a bare mode that will setup the repo without any bells and whistles.

@felixfbecker
Copy link

felixfbecker commented Sep 12, 2016

So it is more like an end-to-end test for typings. But e2e tests in opposite to unit tests do not test a unit-under-test in isolation, in this case we would not only test our declarations, we would also test the implementation (which is already covered by the library tests) and the source documentation (in case examples we copied were wrong). Both don't really belong into our domain necessarily. But I admit for catching the wrong type of import it is useful.

For new versions, you of course have to update the unit tests with new examples from the docs or the library tests. But you have to do that for source tests too.

@unional
Copy link
Collaborator Author

unional commented Sep 12, 2016

Not really e2e, only API test. The tests must be still able to run in isolation.

we would also test the implementation (which is already covered by the library tests) and the source documentation (in case examples we copied were wrong)

That is exactly why I think of re-using the source's test.

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

No branches or pull requests

3 participants