Bug 228040

Summary:

[JSC] Simplify sampling-profiler-regexp.js test

Product:

WebKit

Reporter:

Yusuke Suzuki <ysuzuki>

Component:

New Bugs

Assignee:

Yusuke Suzuki <ysuzuki>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

saam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	saam: review+
Patch	none

Yusuke Suzuki

Reported 2021-07-16 15:59:39 PDT

[JSC] Simplify sampling-profiler-regexp.js test

Attachments
Patch (3.94 KB, patch) 2021-07-16 16:01 PDT, Yusuke Suzuki	saam: review+	Details Formatted Diff Diff
Patch (3.91 KB, patch) 2021-07-16 16:06 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Yusuke Suzuki

Comment 1 2021-07-16 16:01:10 PDT

Created attachment 433712 [details] Patch

Saam Barati

Comment 2 2021-07-16 16:03:35 PDT

Comment on attachment 433712 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433712&action=review > JSTests/stress/sampling-profiler/samplingProfiler.js:37 > +const VERBOSE = true; id you want to check this part in? > JSTests/stress/sampling-profiler/samplingProfiler.js:69 > if (isRunFromRunTest) > - stackTrace = [...stackTrace, "runTest", "(program)"]; > + stackTrace = [...stackTraceOrString, "runTest", "(program)"]; > else > - stackTrace = [...stackTrace]; > + stackTrace = [...stackTraceOrString]; does this make sense if the stack trace is a string?

Yusuke Suzuki

Comment 3 2021-07-16 16:04:32 PDT

Comment on attachment 433712 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433712&action=review >> JSTests/stress/sampling-profiler/samplingProfiler.js:37 >> +const VERBOSE = true; > > id you want to check this part in? Oops! Nice catch. We should make it false. >> JSTests/stress/sampling-profiler/samplingProfiler.js:69 >> + stackTrace = [...stackTraceOrString]; > > does this make sense if the stack trace is a string? We have `if (typeof stackTraceOrString === 'string') {` branch before this :)

Yusuke Suzuki

Comment 4 2021-07-16 16:06:21 PDT

Created attachment 433713 [details] Patch

Saam Barati

Comment 5 2021-07-16 16:07:39 PDT

Comment on attachment 433712 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433712&action=review >>> JSTests/stress/sampling-profiler/samplingProfiler.js:37 >>> +const VERBOSE = true; >> >> id you want to check this part in? > > Oops! Nice catch. We should make it false. 👍 (I meant to write "did" for the first word here) >>> JSTests/stress/sampling-profiler/samplingProfiler.js:69 >>> + stackTrace = [...stackTraceOrString]; >> >> does this make sense if the stack trace is a string? > > We have `if (typeof stackTraceOrString === 'string') {` branch before this :) 👍 I missed that the above branch always returns.

EWS

Comment 6 2021-07-16 17:15:39 PDT

Committed r280011 (239753@main): <https://commits.webkit.org/239753@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 433713 [details].

Radar WebKit Bug Importer

Comment 7 2021-07-16 17:16:22 PDT

<rdar://problem/80709969>

Note You need to log in before you can comment on or make changes to this bug.