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

Bug: filterset binary operators are parsed as function applications #1816

Open
thoughtpolice opened this issue Oct 22, 2024 · 1 comment
Open
Labels
bug Something isn't working

Comments

@thoughtpolice
Copy link

Description of the issue

Description: When running the Jujutsu test suite, I'm seeing a flaky test I would like to skip. I tried using the following command after skimming the filterset documentation:

austin@GANON:~/src/jj$ cargo nextest run --workspace -j 6 -E 'not(test(test_shallow_commits_lack_parents))'

However, this fails with a rather confusing error, because the not operator is (I am assuming completely based on no reading of the code) parsed as a function not() which doesn't exist.

Simply adding a space not (...) in the predicate does what I expect and succeeds.

Expected outcome

I expect the test suite to run, and skip the singular test named test_shallow_commits_lack_parents

Actual result

  error: expected expression
   ╭────
 1 │ not(test(test_shallow_commits_lack_parents))
   · ──────────────────────┬─────────────────────
   ·                       ╰── missing expression
   ╰────

  error: expected end of expression
   ╭────
 1 │ not(test(test_shallow_commits_lack_parents))
   · ──────────────────────┬─────────────────────
   ·                       ╰── unparsed input
   ╰────

error: failed to parse filterset

Nextest version

austin@GANON:~/src/jj$ cargo nextest --version
cargo-nextest 0.9.79
release: 0.9.79
host: x86_64-unknown-linux-gnu

Additional context

I assume this is some kind of grammar nit, though I'm no expert in parsing, but ideally there should be:

  1. Some fix to the grammar to allow this (i.e. parsing binary operators as a separate term). Maybe you could hack it by also defining every binary operator as a 1-arity function :)
  2. At least some documentation on this, if nothing else.
  3. A better error message? I puzzled on this for a solid 5 minutes. Not sure what can be done here.

Also, I note that nextest 0.9.80 added (back) support for --skip, presumably in terms of a filterset-under-the-hood. That would have let me work around this, but this still remains.

@thoughtpolice thoughtpolice added the bug Something isn't working label Oct 22, 2024
@sunshowers
Copy link
Member

Ahh that's definitely confusing. I'm definitely tempted to tokenize something like not separately, but I'm worried people are going to be led down an incorrect path, and start thinking of all the boolean operators as prefix operators. A better error message is definitely valuable here.

And yes, --skip now works :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants