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

test: use ses-ava #9026

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

test: use ses-ava #9026

wants to merge 2 commits into from

Conversation

erights
Copy link
Member

@erights erights commented Mar 2, 2024

closes: #XXXX
refs: endojs/endo#2109

Description

Switch many uses of raw ava to use @endo/ses-ava instead. Done in anticipation of endojs/endo#2109 , which will make the ses-ava experience more pleasant. But not dependent on it, and should work now. Note the absence of an #-endo-branch at the top of this PR comment.

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

none

Testing Considerations

once synced with an endo that includes endojs/endo#2109 , will make for a more pleasant testing and debugging experience.

Upgrade Considerations

none

@erights erights self-assigned this Mar 2, 2024
@erights erights marked this pull request as ready for review March 2, 2024 03:15
@erights erights force-pushed the markm-use-ses-ava branch from 947b30b to faf4656 Compare March 3, 2024 19:10
@erights erights requested review from turadg and removed request for kriskowal March 3, 2024 19:10
@@ -0,0 +1,7 @@
import '@endo/init/debug.js';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any reason for upgrade tests to run under lockdown. They're scripting the chain which is fully locked down. Please leave these as is so we don't have this extra boilerplate to write the tests.

Comment on lines +9 to +10
// eslint-disable-next-line import/order
import { test } from '../tools/prepare-test-env-ava.js';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please avoid introducing a lint suppression.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from fixing endojs/endo#1467 , how? See previous discussion at endojs/endo#1439 (comment)

Copy link
Member

@turadg turadg Mar 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that seems to be the only way. Out of scope, I suppose.

At the least please use // eslint-disable-line so that reordering imports doesn't break the association of the comment with the line. Also would be good to indicate this is not to be emulated by linking to the issue.

Suggested change
// eslint-disable-next-line import/order
import { test } from '../tools/prepare-test-env-ava.js';
import { test } from '../tools/prepare-test-env-ava.js'; // eslint-disable-line import/order -- https://github.com/endojs/endo/issues/1467

@@ -0,0 +1,7 @@
import '@endo/init/debug.js';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this file need to be repeated in every package?

Shouldn't @endo/ses-ava take care of this? E.g. in a test you can simply import,

import test from '@endo/ses-ava';

This is how Ava works. I don't see why ses-ava needs to work so differently.

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

Successfully merging this pull request may close these issues.

2 participants