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 type-level tests for launchTestNode #2775

Open
nedsalk opened this issue Jul 16, 2024 · 5 comments
Open

Add type-level tests for launchTestNode #2775

nedsalk opened this issue Jul 16, 2024 · 5 comments
Labels
chore Issue is a chore

Comments

@nedsalk
Copy link
Contributor

nedsalk commented Jul 16, 2024

Conceptually, you can introduce changes to a type that would still allow the compiler to compile the code successfully but that would introduce a regression as the type is not the intended one. The only way to guard against that is via type-level tests which assert that, for a given set of concrete inputs, the type inference logic returns the exact expected outputs. These type-level tests would be "green" if the compiler successfully compiles them.

We need such type-level tests so that we can catch problems like the one fixed in #2756.

@maschad
Copy link
Contributor

maschad commented Jul 16, 2024

I would like to see a possible instance of such a regression, the scenario in #2756 cannot be re-introduced as it stands as the tagged union was necessarily narrowed , and should someone try to revert it to a tagged union, the compiler would throw an error on the contract function calls.

Writing a test to catch problem such as the one in #2756 would be the equivalent of trying to do a contract function call on the tagged union, at which point the compiler would complain. My apprehension here is that we are doing double work, when we should just leverage the compiler.

I only see such a test offering a benefit when interacting with javascript directly.

@arboleya arboleya added p2 and removed p1 labels Jul 19, 2024
@arboleya arboleya removed this from the 0.x post-launch milestone Jul 19, 2024
@arboleya arboleya added p2 and removed p2 labels Jul 20, 2024
@arboleya arboleya removed the p2 label Aug 2, 2024
@danielbate
Copy link
Member

danielbate commented Aug 15, 2024

Do we still need this given #2579 and #2811 and our increase in test node usage across the repo? How come the test in #2756 was reverted?

@nedsalk
Copy link
Contributor Author

nedsalk commented Aug 15, 2024

@danielbate the problem is that we're not doing compile-time checks in our test suite, hence the usage across the repo doesn't prevent us from introducing a breaking change in the types. We either do that or we create specific tests that we'd run tsc on and where we're asserting that the types are those that we expect. I just created #2941 where type-level tests would be of clearer value. If we introduce type testing infrastructure in that issue, it could be used here as well.

@danielbate
Copy link
Member

Why do we need a separate issue for #2941. Would be not do #2941 as part of this issue? Prove #2941 through type level tests?

@nedsalk
Copy link
Contributor Author

nedsalk commented Aug 15, 2024

@danielbate #2941 modifies the typegen outputs and is unrelated to this issue except that they both touch on type-level tests. This one is related to launchTestNode specifically, whereas #2941 is related to Interface, Contract, Predicate, Script and possibly some others. If it's a lot of clutter, we can close this issue and cover it in #2941.

@arboleya arboleya added temp-p2 and removed temp-M labels Sep 7, 2024
@arboleya arboleya added the temp:notion label Sep 8, 2024 — with Linear
@arboleya arboleya added the temp-linear label Sep 8, 2024 — with Linear
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Issue is a chore
Projects
None yet
Development

No branches or pull requests

4 participants