-
Notifications
You must be signed in to change notification settings - Fork 34
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
docs: fix readme test docs, and add test comments to makefile #225
Conversation
litt3
commented
Jan 8, 2025
- These doc updates are being made in advance of the next proxy release
…s to makefile Signed-off-by: litt3 <[email protected]>
Signed-off-by: litt3 <[email protected]>
@litt3 guessing this is no longer a draft given that you requested reviews? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. unsure about the naming e2e-test though, left a comment. But don't want to block the release for a small thing like this, so approving.
|
||
### Holesky | ||
End-to-end (E2E) tests can be ran via `make e2e-test`. These tests use the [op-e2e](https://github.com/ethereum-optimism/optimism/tree/develop/op-e2e) framework for asserting correct interaction behaviors with batch submission and state derivation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we switch from calling the make target optimism-test
to e2e-test
? I'm guessing these e2e test also test some nitro stuff? If not, should we just change the name back to test-optimism-integration
? cc @epociask
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we initially had a separate entry-point (i.e, OP env flag) for these tests independent of the other E2E tests. Now e2e-test
is a superset of optimism-test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test some nitro stuff
Nop we currently don't do any E2E nitro tests in proxy directly. Could be worth exploring in the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's also add an additional section specifying that these tests also assert E2E client <-> server interactions using simple/op clients
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added verbatim c5d6f0d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice - just one comment 🙏🏻
|
||
### Holesky | ||
End-to-end (E2E) tests can be ran via `make e2e-test`. These tests use the [op-e2e](https://github.com/ethereum-optimism/optimism/tree/develop/op-e2e) framework for asserting correct interaction behaviors with batch submission and state derivation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's also add an additional section specifying that these tests also assert E2E client <-> server interactions using simple/op clients
Signed-off-by: litt3 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM