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

Jump to bottom

go/parser: TestScopeDepthLimit unnecessarily disabled on js/wasm #53816

Closed
mknyszek opened this issue Jul 12, 2022 · 1 comment
Closed

go/parser: TestScopeDepthLimit unnecessarily disabled on js/wasm #53816

mknyszek opened this issue Jul 12, 2022 · 1 comment
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-JS Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.19

Comments

@mknyszek
Copy link
Contributor

It was disabled out of an abundance of caution, but the test can probably just be re-enabled. I tried running it locally and it worked fine. TestParseDepthLimit definitely cannot be trivially re-enabled.

CC @golang/security
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 12, 2022
@mknyszek mknyszek added this to the Backlog milestone Jul 12, 2022
@dmitshur dmitshur added arch-wasm WebAssembly issues OS-JS labels Jul 12, 2022
@bcmills bcmills added the Testing An issue that has been verified to require only test changes, not just a test failure. label Jul 12, 2022
@gopherbot
Copy link

Change https://go.dev/cl/417657 mentions this issue: go/parser: TestParseDepthLimit for short tests

@dmitshur dmitshur modified the milestones: Backlog, Go1.19 Jul 14, 2022
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 14, 2022
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@rolandshoemaker @jproberts
go/parser: skip TestParseDepthLimit for short tests
bc11955 
Because it requires a not insignificant amount of memory to run. Also
remove the WASM-only skip from TestScopeDepthLimit, which is less
intensive.

Fixes golang#53816

Change-Id: I8463046510ce4dd3d8f6d66938828d5e6963c3be
Reviewed-on: https://go-review.googlesource.com/c/go/+/417657
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
@golang golang locked and limited conversation to collaborators Jul 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-JS Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Go1.19
Development

No branches or pull requests
4 participants
@mknyszek @dmitshur @bcmills @gopherbot