test: lint Ava tests

[turadg]

closes: #4941

Description

Motivation in #4941.

• Adds https://github.com/avajs/eslint-plugin-ava
• Disabled use-test due to Allow importing AVA as anyTest in TypeScript files avajs/eslint-plugin-ava#294 (comment)
• Selectively disabled no-lone-blocks that started erroring, I suppose because a dep that was updated caught more cases. Alternately we could disable the rule.
• A test that imported from another test required refactoring

Security Considerations

--

Documentation Considerations

--

Testing Considerations

--

[turadg]

Wrinkle with https://github.com/eslint/eslint/issues/3458 because this makes packages that import @agoric eslint config now have to also import eslint-plugin-ava.

If we're going to have an eslint config that gets imported by packages outside this repo then I suppose it should be minimal and very general. Then more specific ones that can be opted into. For example, sdk and dapp. The ava linting would only be on the 'sdk' one.

So I'll change this PR to

• define that "sdk" config
• use it in all the SDK packages
• make that config lint Ava

[Chris-Hibbert]

with caveats, as discussed.

[Chris-Hibbert]

Should this PR

closes: #6766

?

[turadg]

sadly, no. It's still failing https://github.com/Agoric/agoric-sdk/actions/runs/5867547993/job/15908602617?pr=4981

I'll revert this change

[Chris-Hibbert]

Is this required because of the switch/case? If so, I think the rule is being too aggressive. I wouldn't want to have to add this caveat every time I use swtch/case. Doesn't our style recommend using braces with switch/case? (It should, IMHO.)

[turadg]

Remnant of an earlier revision. I'll remove

[Chris-Hibbert]

remark: Good extraction!

[FUDCo]

I think no-lone-blocks is a dumb rule that serves no useful purpose that I can discern. We should disable it universally.

[FUDCo]

This all looks quite reasonable to me.

[Chris-Hibbert]

I think no-lone-blocks is a dumb rule that serves no useful purpose that I can discern. We should disable it universally.

I don't know why it's a good rule to enforce, but I've found that my tastes don't seem to prevail unless I can show why a proposed rule would get in the way. I asked enough questions to convince myself that it doesn't prevent anything that I'm aware of doing. @FUDCo is there something that you occasionally do that this would grouse about?

[FUDCo]

I think the example you already cited of switch cases is compelling by itself, but also: sometimes lone blocks are useful for expository purposes to highlight functionally distinct parts of a function, much as blank lines are used. Also, I've occasionally found them useful to control what gets captured by a closure -- sometimes optimizations inside the JS engine can result in more stuff getting captured by a closure than you want or than is lexically apparent from reading the code; arguably this latter case is more about working around JS engine issues, but real nonetheless.