Skip to content

Implicit braces in ocamltest trees #2693

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lukemaurer
Copy link
Contributor

The new tree syntax for ocamltest greatly improves the common case where several test cases are sequentially composed, each depending on the last. We can now write such tests as

test1;
test2;
test3;
test4;
...
test27;

without increasing the indentation level. This makes sense because these tests are not “nested” in any meaningful sense, any more than consecutive statements in imperative code are nested. Practically speaking, the great advantage is that we can now add test1a between test1 and test2 without increasing indentation (or other markers of nested-ness).

Unfortunately, this only works if test1a belongs in the sequence with the other tests. But it might be completely independent: for example, it might check an error case that is only meaningful after test1 but has no bearing on the other tests. In that case, we have three unpalatable options:

  1. Write it in the sequence anyway. This obscures the structure of the test cases and causes the entire rest of the sequence to be skipped if test1a fails.
  2. Indent every test in the sequence after test1a and risk emotional damage the next time you try to rebase.
  3. Add braces around every test in the sequence after test1a but don't indent, then try to invent a convention that will help you keep the formatting straight.

The annoying thing here is that test4 is still not meaningfully “more nested” than test1. It just happens that there's an extra branch jutting out of the sequence somewhere between test1 and test4, and the syntax makes that everyone's problem.

My proposed syntax is this:

test1;
{
 test1a;
}
test2;
test3;
test4;
...
test27;

This is exactly as if we'd put braces around everything from test2 to test27, but it once again expresses that this is simply a sequence of tests in a chain. It's just that now there are some tests that branch off from the sequence.

The new tree syntax for ocamltest greatly improves the common case where several
test cases are sequentially composed, each depending on the last. We can now
write such tests as

```
test1;
test2;
test3;
test4;
...
test27;
```

without increasing the indentation level. This makes sense because these tests
are not “nested” in any meaningful sense, any more than consecutive statements
in imperative code are nested. Practically speaking, the great advantage is that
we can now add `test1a` between `test1` and `test2` without increasing
indentation (or other markers of nested-ness).

Unfortunately, this only works if `test1a` belongs in the sequence with the
other tests. But it might be completely independent: for example, it might check
an error case that is only meaningful after `test1` but has no bearing on the
other tests. In that case, we have three unpalatable options:

1. Write it in the sequence anyway. This obscures the structure of the test
   cases and causes the entire rest of the sequence to be skipped if `test1a`
   fails.
2. Indent _every test in the sequence after `test1a`_ and risk emotional damage
   the next time you try to rebase.
3. Add braces around every test in the sequence after `test1a` but don't indent,
   then try to invent a convention that will help you keep the formatting
   straight.

The annoying thing here is that `test4` is _still_ not meaningfully “more nested”
than `test1`. It just happens that there's an extra branch jutting out of the
sequence somewhere between `test1` and `test4`, and the syntax makes that
everyone's problem.

My proposed syntax is this:

```
test1;
{
 test1a;
}
test2;
test3;
test4;
...
test27;
```

This is exactly as if we'd put braces around everything from `test2` to
`test27`, but it once again expresses that this is simply a sequence of tests in
a chain. It's just that now there are some tests that branch off from the
sequence.
@stedolan
Copy link
Contributor

I like this feature and the implementation is certainly straightforward!

But this is an area that I'd really prefer to avoid diverging from upstream OCaml, because resolving ocamltest conflicts is already annoying. Could you make this as an upstream PR and see if it gets accepted there, then backport?

@lukemaurer
Copy link
Contributor Author

I like this feature and the implementation is certainly straightforward!

But this is an area that I'd really prefer to avoid diverging from upstream OCaml, because resolving ocamltest conflicts is already annoying. Could you make this as an upstream PR and see if it gets accepted there, then backport?

Sure, will do

@lukemaurer
Copy link
Contributor Author

Submitted upstream as ocaml/ocaml#13259

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants