Some ses-ava static test methods don't work

[erights]

Changing test-message-pattern.js to use ses-ava causes the test.serial at
https://github.com/Agoric/agoric-sdk/blob/201cd2d4ed41a41bb9b751a714f99fcd3f6bb303/packages/SwingSet/test/test-message-patterns.js#L123

to fail at
https://github.com/Agoric/agoric-sdk/pull/2740/checks?check_run_id=2211691217#step:7:220

  Uncaught exception in test/test-message-patterns.js

  test/test-message-patterns.js:123

   122:   }                                                     
   123:   test.serial('local patterns', testLocalPattern, name);
   124: }                                                       

  Error: Duplicate test title: local patterns

[erights]

Also test.after.always. See https://github.com/Agoric/agoric-sdk/pull/2740/checks?check_run_id=2214861371#step:10:10

[erights]

I'm listing methods in this issue as I encounter them in trying to switch to ses-ava.
If it is easier to fix our code to not use these methods then it is to get ses-ava to virtualize these correctly, feel free. I'd rather define ses-ava to implement a sensible subset of the ava API if possible.

[FUDCo]

I'm very confused. First, there's no test.repeat there. Second, my change to ses-ava last week makes this work. Something has gotten tangled up in the plumbing somewhere.

[erights]

Doh! I remembered the wrong name. test.serial is the problem. I revised the first message in this thread to correct that.

[FUDCo]

OK, that takes care of my first point, but why did this work correctly for me the other day? The change I made to ses-ava should (did!) fix this.

[erights]

It seems to be only those two uses of test.serial. Several others do fine with ses-ava. Some others have not yet been tried with ses-ava.

[FUDCo]

That specific test file (test-message-patterns.js) was the thing I was going on about wanting to run under ses-ava and after my changes to ses-ava to support the Ava "macro" facilitiy, it worked. So if it doesn't work now, something else you did changed something to make it not work. I'm trying to figure out what that might have been.

[erights]

The left pane of

https://github.com/Agoric/agoric-sdk/pull/2740/files#diff-8845f7584737c882a225a4b88eee42f5064626370ad4b222d3e0319fd6e4cb68

shows that it was previously (prior to this PR at least) using ava, not ses-ava. When was it using ses-ava ?

[FUDCo]

To use it with ses-ava I had to checkout your SES 12.5 update branch and manually patch the test. I put the test back the way it was before committing the SwingSet changes I was testing, because at the time checking in the version of the test that used ses-ava would break CI; I was waiting for the SES 12.5 changes, as well as my changes to ses-ava itself, to land so I could put the test back in a state to use ses-ava, but the latter changes are essentially what you have now done here. So the mystery is why it doesn't now work, since it worked before. The diff you linked to is the diff to test-message-patterns.js, but that's not what I was asking about. I was asking what you'd changed in ses-ava to cause it to stop working in this case.

[erights]

https://github.com/Agoric/dapp-token-economy/pull/154/files
is the only change to ses-ava since your "macro" change.
See https://github.com/endojs/endo/commits/master/packages/ses-ava
Nothing there but updating dependency on ses to 0.12.6 and one irrelevant test change.

[FUDCo]

OK, I think I figured it out. It looks like the changes to ses-ava haven't been released. The version that a yarn install installs predates my fix.

[FUDCo]

Let's revisit this. I think it can be closed.

[erights]

Close as in "it's already fixed, let's close the bug" or "let's fix this"?

[FUDCo]

In this case, I think "already fixed"

[erights]

Looks like the bug report was provoked by test-message-pattern.js breaking when we tried switching it to ses-ava. If the underlying issue is fixed, can we now switch test-message-pattern.js to ses-ava? Can we make that switch as part of closing this bug?

[erights]

Looks like we've got over 180 imports of 'ava' rather than 'ses-ava', most of which I expect should be switched if these are no longer problems.

Again, not an MN-1 issue. But definitely a real problem!

[erights]

@turadg added you as you're interesting in such improvements. Should this one be closed as a dup?

[turadg]

I closed the other one since this describes a more general problem.

[erights]

@turadg , can we close this as fixed by your Agoric/agoric-sdk#5774 ? Is there any remaining reason it needs to stay open?

[erights]

Hi @turadg , I think this is fixed, so I'm closing. Please reopen if I'm missing something. Thanks.

[turadg]

That was an SDK PR but I can confirm this is fixed in Endo now too.