fix: tests use debug settings#1213
Conversation
| { | ||
| message: | ||
| "checkBundle cannot bundle without the property 'endoZipBase64Sha512', which must be a string, got (a string)", | ||
| 'checkBundle cannot bundle without the property \'endoZipBase64Sha512\', which must be a string, got "undefined"', |
There was a problem hiding this comment.
This was the case causing the failure.
|
I left one |
4e39ef7 to
880fa03
Compare
I have since changed my mind, and the PR no longer makes a special case for |
| // Use a package self-reference to go through the "exports" resolution | ||
| // eslint-disable-next-line import/no-extraneous-dependencies | ||
| import '@endo/init'; | ||
| import '@endo/init/debug.js'; |
There was a problem hiding this comment.
Reviewers, since this is in packages/init, I'd like feedback on whether this change is appropriate before merging.
There was a problem hiding this comment.
If debug transitively imports pre throught the package.json module map, i think it's ok
There was a problem hiding this comment.
Thinking about it, it should since it's a public entry point itself.
0c0beb3 to
563d020
Compare
I normally run with
export LOCKDOWN_ERROR_TAMING=unsafeThis caused a failure in a test case comparing error messages, that were made more explanatory with this setting. The problem is that our tests were run with a neutral
lockdowncall that was letting this setting be set by the environment variable, but the test itself happened to depend on this option being'safe'. I think our best practice should be thattest-*.jsava test files[1] should use the@endo/init/debug.jssettings rather than the@endo/initsettings. Then the error messages during testing will be more informative without endangering info leakage in production.[1] aside from those whose purpose is to test the lockdown options, or whose purpose is to test that sensitive info in error messages get properly redacted.