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

Implement test steps API #11901

Closed
1 of 5 tasks
dsherret opened this issue Sep 2, 2021 · 12 comments
Closed
1 of 5 tasks

Implement test steps API #11901

dsherret opened this issue Sep 2, 2021 · 12 comments
Assignees
Labels
cli related to cli/ dir testing related to deno test and coverage

Comments

@dsherret
Copy link
Member

dsherret commented Sep 2, 2021

We're going to start work on a prototype of the test steps API.

@dsherret dsherret added cli related to cli/ dir testing related to deno test and coverage labels Sep 2, 2021
@dsherret dsherret self-assigned this Sep 2, 2021
@sebastienfilion
Copy link
Contributor

The RFC you link (#10771) is a long conversation with a lot of contracting concepts. Which one exactly do you propose to implement?
Also, can you define what "metadata" are you referring to?

@dsherret
Copy link
Member Author

dsherret commented Sep 2, 2021

What's outlined in the main post. See stepMetadata property.

@sebastienfilion
Copy link
Contributor

Are you going to implement the Tester class? Otherwise, I don't understand to what end.
Also this post is confusing; are you just saying that we're going to implement everything and see what sticks?

@dsherret
Copy link
Member Author

dsherret commented Sep 2, 2021

We're implementing what's outlined in the RFC at #10771 (so yes, because the Tester class is outlined there). Then we're going to implement a pollyfill for #11235 that uses the implementation of #10771. After we'll make further decisions on where the #11235 approach will live (it might end up being avialble in the CLI, std, or even in deno.land/x. In the case of it being in the CLI then we'll drop the stepMetadata property, but all of this isn't decided yet). We're going to make these decisions before merging the end result.

@caspervonb
Copy link
Contributor

After we'll make further decisions on where the #11235 approach will live (it might end up being avialble in the CLI, std, or even in deno.land/x. In the case of it being in the CLI then we'll drop the stepMetadata property, but all of this isn't decided yet)

This is fairly confusing.
So we're doing declarative test registration that runs imperative tests that then can be used to registers declarative tests again?

@dsherret
Copy link
Member Author

dsherret commented Sep 2, 2021

So we're doing declarative test registration that runs imperative tests that then can be used to registers declarative tests again?

This is a temporary implementation detail. No decision has been made in this regard (as stated above, we will make a decision on this later).

@caspervonb
Copy link
Contributor

caspervonb commented Sep 3, 2021

This is a temporary implementation detail

Okay sure but it's a big one.

The style of the API doesn't really matter at all, but when the tests are registered and executed does since we have already established a test runner with deferred execution.

e.g, in e.g in https://gist.github.com/caspervonb/62db35a0cc9532dc618b96fcda205576 the convenience function provides you with an imperative style but it is still deferred execution with the same semantics as top level tests.
It's quite literally just a nested test harness.

Immediately executing user controlled tests still fundamentally clash with how regular top level tests actually work with deferred runner controlled and throttled execution so I'm very confused as to how we are planning on making deferred test execution live in deno.land/std or deno.land/x.

Are we throwing out the test runner completely now and going immediate execution all the way? This seems to be really at odds with what I've been building out for the past few months.

We can do immediate steps with reporting and that can live anywhere but we can't build TestDefinitions on immediate execution without fundamentally changing how the test runner works. If the test runner isn't in charge of scheduling and executing the tests why is it in the test runner? Without that we're just calling functions.

Both proposals leave many questions open, including but not limited to:

  • How does concurrent_jobs tie into all of this and how do you disable concurrency for debugging a failed test?
  • How do you run setup and teardown for top level tests?
  • How do you run top level tests concurrently?

@caspervonb
Copy link
Contributor

caspervonb commented Sep 3, 2021

Just to simplify and clarify my main concern which is the ability to control execution in the test runner.
Are the execution of subtests in this metadata thing deferred and controlled by the test runner or are they immediate and executed by user?

@lucacasonato
Copy link
Member

  • The step metadata only affects filtering (and maybe display). It can not be used to provide things like permissions.
  • Metadata can only be present on top level Deno.test registrations.
  • Subtests are invoked when t.step is called. The returned future resolves when the test is complete. We can delay the actual invocation after calling t.step if necessary for whatever reason, because of the returned Promise.

@caspervonb
Copy link
Contributor

caspervonb commented Sep 6, 2021

Thanks @lucacasonato, so this is still just test steps that's slightly less confusing.

Altho still see no practical use for metadata and don't understand it's purpose as we can filter just fine without it.
As I see no use for it nor any strong arguments for having it, I am against including it.


Write polyfill for declarative in CLI RFC: Test Collections—Declarative approach to subtests #11235
Decide on declarative living in CLI, std, or /x

This bit is still confusing tho as one just cannot have tightly integrated deferred execution in user land.
Test steps and say #11789 can co-exist just fine but #11789 can't be built on test steps in deno.land/std or deno.land/x.

@bartlomieju
Copy link
Member

@dsherret can this issue be closed now?

@dsherret
Copy link
Member Author

@bartlomieju yeah, we can decide on next steps later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir testing related to deno test and coverage
Projects
None yet
Development

No branches or pull requests

5 participants