Add setUp and tearDown to simplify tests.

[hayd]

This also adds a main.ts which allows one to to grep
through tests. Ideally this would recursively import
all test.ts or *_test.ts files.

The idea was that this could later be called by deno test.

Note: The first commit is a small cleanup after #105.

[ry]

We really need a better way to do this...

[hayd]

I wonder if what you want is:

isReady(() => bool)
timeout: number

A waitUntil async funtion could check isReady every second (?) until timeout.

[hayd]

This can be done pretty easily with these utils:

setUp(async function startFileServer() {
  fileServer = run({
    args: ["deno", "--allow-net", "http/file_server.ts", ".", "--cors"]
  });
  // Wait for fileServer to spool up.
  // await new Promise(res => setTimeout(res, 1000));
  await waitUntil(async function() {
    try { await fetch("http://localhost:4500/"); return true }
    catch(e) { return false } ;
  }, 1000)
});

// given
async function waitUntil(isReady: ()=>boolean|Promise<boolean>, timeout: number): Promise<void> {
  let t = 0;
  while (!await isReady()) {
    await delay(1000);
    if (t++ > timeout) {
      throw Error(`Timeout waiting for ${isReady}`)
    }
  }
}

async function delay(ms: number): Promise<void> {
  await new Promise(res => setTimeout(res, ms));
}

do waitUntil and delay belong in an std as standalone?

I can update this PR...

Edit: I suspect until is a better name than waitUntil.

[ry]

I think a better way to wait for file_server to be ready is to read from its stdout and wait until you see "HTTP server listening on http://0.0.0.0:4500/"

[hayd]

Fixed.

[ry]

There's no guarantee that the read will get an entire line. So this is not correct.
I suggest using something like this (untested)

async function waitForFileServerReady(fileServer: deno.Process): Promise<bool> {
  let stdout = new TextProtoReader(fileServer.stdout);
  let err = null;
  while (!err) {
    let [line, err] = await r.readLine();
    if (line.includes("server listening")) return true;
  }
  return false;
}

[ry]

(As a warning, it may be that TextProtoReader only works on \r\n line breaks... but try this first..)

[ry]

See this example also
https://github.com/denoland/deno_std/blob/9f1de43cc47b0bee4d7d04a1b3ebc238333575bc/textproto/test.ts#L15-L28

[hayd]

Does this look right?

setUp(async function startFileServer() {
  fileServer = run({
    args: ["deno", "--allow-net", "http/file_server.ts", ".", "--cors"],
    stdout: "piped"
  });

  const r = new TextProtoReader(new BufReader(fileServer.stdout));
  const [s, err] = await r.readLine();
  assert(err == null);
  assert(s.includes("server listening"))
});

[ry]

Yes, but fileServer needs to have fileServer.close() and fileServer.stdout.close() called on it during tearDown

[hayd]

fileServer.close() is in tearDown.
That doesn't imply fileServer.stdout.close() ?

[hayd]

Fixed.

[ry]

While you're fixing this, can we cat a smaller file? I've noticed this is flooding the log

[hayd]

The piped stops it flooding the log.

[ry]

👍

[ry]

Can we just fix timer for starting fileServer in this PR and leave out the setUP and tearDown stuff?

Because modifying testing will effect all of deno, this needs to be considered very carefully. I'm not sure I agree with setting a global setUp/tearDown.

Please just manually do the setUP and tearDown for each of the fileServer tests with the new initialization. After that's landed let's discuss these testing changes (in gitter if you have time)

[hayd]

I'm not sure I agree with setting a global setUp/tearDown.

I would prefer this was per-file, but am not sure on the trick to make this work.

I also want to improve this across all of deno / have it considered carefully.

I could tease it apart:

• timing of fileserver
• main.ts for testing
• setUp + tearDown (to discuss)

[ry]

timing of fileserver

Let's do just this in this PR.

main.ts for testing

Yeah. I think we should get rid of the "automatic" run behavior that we currently have
https://github.com/denoland/deno_std/blob/b99d7d3e0f61216db7b5a5ad2bc254f8ac97ff4d/testing/mod.ts#L166
and rather require users to call runTests() explicitly.

[ry]

@hayd I'm very interested in having your improvements here. I do think they should be split up into different PRs tho. For simplicity, I'm going to close this one.

[hayd]

I wonder if an alternative for the setUp/tearDown (or setup/teardown) here is to have test take these as options arguments. That way if you did want every test to wrap with the same setup/teardown you could define your own partial function.

Edit: i.e.

test(function foo() {...}, { setup: setup, teardown: teardown })
// is equivalent to
test({
  setup()
  try {
    foo()
  } finally {
    teardown()
  }
})

[hayd]

An alternative might look like this: hayd@987ef51