Saqlain (Gerrit)

unread,

Jun 14, 2025, 4:29:38 PMJun 14

to Chromium LUCI CQ, Adam Raine, chromium...@chromium.org, blink-...@chromium.org

Attention needed from Adam Raine and Michael Wilson

Saqlain added 1 comment

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Saqlain . resolved

FYI: `audioparam-testing.js` was not being used in original old test file.

Open in Gerrit

Related details

Attention is currently required from:

Adam Raine
Michael Wilson

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Michael Wilson (Gerrit)

unread,

Jun 17, 2025, 7:29:47 PMJun 17

to Saqlain, Alvin Ji, Adam Raine, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org

Attention needed from Alvin Ji and Saqlain

Michael Wilson added 2 comments

File third_party/blink/web_tests/webaudio/AudioParam/audioparam-sampling.html

Line 107, Patchset 3 (Latest): function assert_close_to(actual, expected, threshold, description) {

Michael Wilson . unresolved

Can we adapt `assert_approx_equals` from testharness.js instead?

https://crsrc.org/c/third_party/blink/web_tests/resources/testharness.js;l=1748

Line 119, Patchset 3 (Latest): const source = context.createBufferSource();

Michael Wilson . unresolved

Let's use `new` here instead.

Open in Gerrit

Related details

Attention is currently required from:

Alvin Ji
Saqlain

Submit Requirements:

Code-Coverage
Code-Review

No-Unresolved-Comments

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Saqlain (Gerrit)

unread,

Jun 18, 2025, 5:01:15 PMJun 18

to Alvin Ji, Adam Raine, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org

Attention needed from Alvin Ji and Michael Wilson

Saqlain added 2 comments

File third_party/blink/web_tests/webaudio/AudioParam/audioparam-sampling.html

Line 107, Patchset 3: function assert_close_to(actual, expected, threshold, description) {

Michael Wilson . resolved

Can we adapt `assert_approx_equals` from testharness.js instead?
https://crsrc.org/c/third_party/blink/web_tests/resources/testharness.js;l=1748

Saqlain

Done

Line 119, Patchset 3: const source = context.createBufferSource();

Michael Wilson . resolved

Let's use `new` here instead.

Saqlain

Done

Open in Gerrit

Related details

Attention is currently required from:

Alvin Ji
Michael Wilson

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Michael Wilson (Gerrit)

unread,

Jun 18, 2025, 5:15:22 PMJun 18

to Saqlain, Alvin Ji, Adam Raine, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org

Attention needed from Alvin Ji and Saqlain

Michael Wilson voted and added 1 comment

Votes added by Michael Wilson

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Michael Wilson . resolved

LGTM

Open in Gerrit

Related details

Attention is currently required from:

Alvin Ji
Saqlain

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Alvin Ji (Gerrit)

unread,

Jun 18, 2025, 6:00:14 PMJun 18

to Saqlain, Adam Raine, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org

Attention needed from Saqlain

Alvin Ji added 2 comments

File third_party/blink/web_tests/webaudio/AudioParam/audioparam-sampling.html

Line 4, Patchset 4 (Latest): <title>Sampling of Linear/Exponential Ramp Automation</title>

Alvin Ji . unresolved

I think we can make this title more precise. `Automation` is a broad concept in WebAudio, whereas this test specifically targets `LinearRampToValueAtTime` and `ExponentialRampToValueAtTime`
How about: `Test Linear-/Exponential-RampToValueAtTime`

Line 157, Patchset 4 (Latest): `Sampling ramp automation: ${config.desc}`);

Alvin Ji . unresolved

Similarly, let's directly mention the tested config here.
`${config.desc} Test`.

Open in Gerrit

Related details

Attention is currently required from:

Saqlain

Submit Requirements:

Code-Coverage
Code-Review

No-Unresolved-Comments

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Saqlain (Gerrit)

unread,

Jun 18, 2025, 6:07:46 PMJun 18

to Alvin Ji, Adam Raine, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org

Attention needed from Alvin Ji and Michael Wilson

Saqlain added 2 comments

File third_party/blink/web_tests/webaudio/AudioParam/audioparam-sampling.html

Line 4, Patchset 4: <title>Sampling of Linear/Exponential Ramp Automation</title>

Alvin Ji . resolved

I think we can make this title more precise. `Automation` is a broad concept in WebAudio, whereas this test specifically targets `LinearRampToValueAtTime` and `ExponentialRampToValueAtTime`
How about: `Test Linear-/Exponential-RampToValueAtTime`

Saqlain

thank you, it makes sense. :)

Line 157, Patchset 4: `Sampling ramp automation: ${config.desc}`);

Alvin Ji . resolved

Similarly, let's directly mention the tested config here.
`${config.desc} Test`.

Saqlain

Done

Open in Gerrit

Related details

Attention is currently required from:

Alvin Ji
Michael Wilson

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Michael Wilson (Gerrit)

unread,

Jun 18, 2025, 6:12:37 PMJun 18

to Saqlain, Alvin Ji, Adam Raine, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org

Attention needed from Alvin Ji and Saqlain

Michael Wilson voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Alvin Ji
Saqlain

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Alvin Ji (Gerrit)

unread,

Jun 20, 2025, 8:02:06 PMJun 20

to Saqlain, Adam Raine, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org

Attention needed from Saqlain

Alvin Ji voted and added 1 comment

Votes added by Alvin Ji

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Alvin Ji . resolved

LGTM

Open in Gerrit

Related details

Attention is currently required from:

Saqlain

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Saqlain (Gerrit)

unread,

Jun 20, 2025, 10:15:09 PMJun 20

to Alvin Ji, Adam Raine, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org

Saqlain voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Jun 20, 2025, 10:18:45 PMJun 20

to Saqlain, Alvin Ji, Adam Raine, chromium...@chromium.org, blink-...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

[webaudio] Migrate audioparam-sampling.html limit test to testharness.js

Convert webaudio/audioparam/set-target-at-time-approach-limit.html from
the legacy audit.js runner to pure testharness.js

Bug: 396477777

Change-Id: I61ef5e2841a28267052baa937e466b026e254ff8

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6632574

Commit-Queue: Saqlain <2mesa...@gmail.com>

Reviewed-by: Alvin Ji <alv...@chromium.org>

Reviewed-by: Michael Wilson <mjwi...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1476854}

Files:

M third_party/blink/web_tests/webaudio/AudioParam/audioparam-sampling.html

Change size: M

Delta: 1 file changed, 79 insertions(+), 93 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Michael Wilson, +1 by Alvin Ji

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

[webaudio] Migrate audioparam-sampling.html limit test to testharness.js [chromium/src : main]

Saqlain (Gerrit)

Saqlain added 1 comment

Related details

Michael Wilson (Gerrit)

Michael Wilson added 2 comments

Related details

Saqlain (Gerrit)

Saqlain added 2 comments

Related details

Michael Wilson (Gerrit)

Michael Wilson voted and added 1 comment

Votes added by Michael Wilson

1 comment

Related details

Alvin Ji (Gerrit)

Alvin Ji added 2 comments

Related details

Saqlain (Gerrit)

Saqlain added 2 comments

Related details

Michael Wilson (Gerrit)

Michael Wilson voted Code-Review+1

Related details

Alvin Ji (Gerrit)

Alvin Ji voted and added 1 comment

Votes added by Alvin Ji

1 comment

Related details

Saqlain (Gerrit)

Saqlain voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information