-
Notifications
You must be signed in to change notification settings - Fork 334
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
Create and use snapshot by default in all wd_py_test
s
#3415
base: main
Are you sure you want to change the base?
Conversation
d9b7619
to
bfddf71
Compare
@@ -21,6 +21,8 @@ py_wd_test("env-param") | |||
py_wd_test( | |||
"asgi", | |||
skip_python_flags = ["0.27.1"], | |||
# TODO: investigate failure! | |||
snapshot_variant = False, |
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.
Just to make it a bit clearer:
snapshot_variant = False, | |
snapshot_generating_variant = False, |
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.
As we spoke over chat.
Let's instead add a variant that generates the snapshot and then uses it to run the test to actually test e2e behaviour.
You all probably know this already, but just to make sure we're on the same page: we already have test coverage for this in EW. The fastest way forward would be to just add the vendored worker sample in there. Of course, creating test coverage for this in workerd is still useful and we should do it (assuming the effort required isn't too massive). |
bfddf71
to
ce54902
Compare
The generated output of |
efc7305
to
f2be084
Compare
Having these unindented lines inside of a conditional block makes the flow control hard to read
f2be084
to
a4781a4
Compare
wd_py_test
swd_py_test
s
a4781a4
to
f6b4c2c
Compare
This isn't as good as having actual validator coverage but it should help catch a lot of the problems we're having at validation time.
On top of #3476.