Michael Wilson (Gerrit)

unread,

Jun 17, 2025, 7:31:36 PM6/17/25

to Punith Nayak, Alvin Ji, Adam Raine, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Alvin Ji and Punith Nayak

Michael Wilson added 1 comment

File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-panner.html

Line 13, Patchset 1 (Latest):

      /*
       * Lightweight helper named `expect`. It mirrors the audit.js helper but
       * relies solely on testharness.js assertions.
       */

Michael Wilson . unresolved

Is it possible to use the assertions directly? If not, we should remove references to audit.js in the comments because we will eventually remove this file.

Open in Gerrit

Related details

Attention is currently required from:

Alvin Ji
Punith Nayak

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

Punith Nayak (Gerrit)

unread,

Jun 19, 2025, 5:54:00 PM6/19/25

to Alvin Ji, Adam Raine, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Alvin Ji and Michael Wilson

Punith Nayak added 1 comment

File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-panner.html

Line 13, Patchset 1: /*


       * Lightweight helper named `expect`. It mirrors the audit.js helper but
       * relies solely on testharness.js assertions.
       */

Michael Wilson . resolved

Is it possible to use the assertions directly? If not, we should remove references to audit.js in the comments because we will eventually remove this file.

Punith Nayak

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 20, 2025, 6:21:41 PM6/20/25

to Punith Nayak, Alvin Ji, Adam Raine, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Alvin Ji and Punith Nayak

Michael Wilson added 6 comments

File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-panner.html

Line 4, Patchset 5 (Latest): <title>Test k-rate AudioParams of PannerNode (direct assertions)</title>

Michael Wilson . unresolved

I don't think we need to say "(direct assertions)" here.

Line 40, Patchset 5 (Parent): doTest(context, should, {

Michael Wilson . unresolved

The original test seems to create two PannerNodes, one at a-rate and one at k-rate, and then checks that they have different output. Please double-check the function `doTest` here:
https://crsrc.org/c/third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/automation-rate-testing.js

I think we're missing the a-rate panner node in the new version.

Line 50, Patchset 5 (Latest): numberOfChannels: 2,

Michael Wilson . unresolved

Is there a reason for changing this from 3 to 2? Please see `doTest` for how the channels were used in the original test.

Line 57, Patchset 5 (Latest): source.offset.value = 1;

Michael Wilson . unresolved

This is the default value, I don't think we need to set it (or we can set it with the options object: https://webaudio.github.io/web-audio-api/#ConstantSourceOptions).

Line 145, Patchset 5 (Latest): source.offset.value = 1;

Michael Wilson . unresolved

This is the default value, I don't think we need to set it (or we can set it with the options object: https://webaudio.github.io/web-audio-api/#ConstantSourceOptions).

Line 199, Patchset 5 (Latest): assert_all_close(

Michael Wilson . unresolved

I think in this case they should actually be equal, not just close.

Let's also add back the comment "Verify that the output is constant over each render quantum"

Open in Gerrit

Related details

Attention is currently required from:

Alvin Ji
Punith Nayak

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

Punith Nayak (Gerrit)

unread,

Jun 24, 2025, 5:46:29 PM6/24/25

to Adam Raine, Alvin Ji, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Alvin Ji and Michael Wilson

Punith Nayak added 6 comments

File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-panner.html

Line 4, Patchset 5: <title>Test k-rate AudioParams of PannerNode (direct assertions)</title>

Michael Wilson . resolved

I don't think we need to say "(direct assertions)" here.

Punith Nayak

Done

Line 40, Patchset 5 (Parent): doTest(context, should, {

Michael Wilson . resolved

The original test seems to create two PannerNodes, one at a-rate and one at k-rate, and then checks that they have different output. Please double-check the function `doTest` here:
https://crsrc.org/c/third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/automation-rate-testing.js
I think we're missing the a-rate panner node in the new version.

Punith Nayak

Done

Line 50, Patchset 5: numberOfChannels: 2,

Michael Wilson . resolved

Is there a reason for changing this from 3 to 2? Please see `doTest` for how the channels were used in the original test.

Punith Nayak

Done

Line 57, Patchset 5: source.offset.value = 1;

Michael Wilson . resolved

This is the default value, I don't think we need to set it (or we can set it with the options object: https://webaudio.github.io/web-audio-api/#ConstantSourceOptions).

Punith Nayak

Done

Line 145, Patchset 5: source.offset.value = 1;

Michael Wilson . resolved

This is the default value, I don't think we need to set it (or we can set it with the options object: https://webaudio.github.io/web-audio-api/#ConstantSourceOptions).

Punith Nayak

Done

Line 199, Patchset 5: assert_all_close(

Michael Wilson . resolved

I think in this case they should actually be equal, not just close.
Let's also add back the comment "Verify that the output is constant over each render quantum"

Punith Nayak

Done

Open in Gerrit

Related details

Attention is currently required from:

Alvin Ji
Michael Wilson

Submit Requirements:

Code-Coverage
Code-Owners

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 24, 2025, 7:03:50 PM6/24/25

to Punith Nayak, Adam Raine, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Michael Wilson and Punith Nayak

Alvin Ji added 3 comments

File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-panner.html

Line 14, Patchset 7 (Latest): if (Math.abs(arr[i] - first) > Number.EPSILON) return; // varied → OK

Alvin Ji . unresolved

Please add the curly bracket for the if statement also make the comment slightly complete.
e.g.
```
    if (Math.abs(arr[i] - first) > Number.EPSILON) {
      // If any element differs from the first by more than a negligible amount,
      // the array is not constant, and the assertion passes.
      return;
    }
```

Line 32, Patchset 7 (Latest):

      const K_RATE = 'k-rate';
      const BLOCK  = 128;

Alvin Ji . unresolved

Consider adding comments to constants.
e.g.
```
  // Represents the 'k-rate' AudioParam automation rate.
  const K_RATE = 'k-rate';
  // Defines the size of one audio processing block (render quantum) in frames.
  const BLOCK = 128;
```

Line 54, Patchset 7 (Latest): const sampleRate = 8000;

Alvin Ji . unresolved

We could also add the SAMPLE_RATE constant if the same sample rate is used across the tests.

Open in Gerrit

Related details

Attention is currently required from:

Michael Wilson
Punith Nayak

Submit Requirements:

Code-Coverage
Code-Owners
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

Punith Nayak (Gerrit)

unread,

Jun 24, 2025, 8:11:15 PM6/24/25

to Alvin Ji, Adam Raine, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Alvin Ji and Michael Wilson

Punith Nayak added 3 comments

File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-panner.html

Line 14, Patchset 7: if (Math.abs(arr[i] - first) > Number.EPSILON) return; // varied → OK

Alvin Ji . resolved

Please add the curly bracket for the if statement also make the comment slightly complete.
e.g.
```
    if (Math.abs(arr[i] - first) > Number.EPSILON) {
      // If any element differs from the first by more than a negligible amount,
      // the array is not constant, and the assertion passes.
      return;
    }
```

Punith Nayak

Done

Line 32, Patchset 7:

      const K_RATE = 'k-rate';
      const BLOCK  = 128;

Alvin Ji . resolved

Consider adding comments to constants.
e.g.
```
  // Represents the 'k-rate' AudioParam automation rate.
  const K_RATE = 'k-rate';
  // Defines the size of one audio processing block (render quantum) in frames.
  const BLOCK = 128;
```

Punith Nayak

Done

Line 54, Patchset 7: const sampleRate = 8000;

Alvin Ji . resolved

We could also add the SAMPLE_RATE constant if the same sample rate is used across the tests.

Punith Nayak

Done

Open in Gerrit

Related details

Attention is currently required from:

Alvin Ji
Michael Wilson

Submit Requirements:

Code-Coverage
Code-Owners
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 25, 2025, 6:58:54 AM6/25/25

to Punith Nayak, Adam Raine, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Michael Wilson and Punith Nayak

Alvin Ji added 1 comment

File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-panner.html

Line 41, Patchset 8 (Latest): const SAMPLE_RATE = 48000;

Alvin Ji . unresolved

Should this be `8000`?

Open in Gerrit

Related details

Attention is currently required from:

Michael Wilson
Punith Nayak

Submit Requirements:

Code-Coverage
Code-Owners
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

Punith Nayak (Gerrit)

unread,

Jun 25, 2025, 7:20:50 AM6/25/25

to Alvin Ji, Adam Raine, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Alvin Ji and Michael Wilson

Punith Nayak added 1 comment

File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-panner.html

Line 41, Patchset 8: const SAMPLE_RATE = 48000;

Alvin Ji . resolved

Should this be `8000`?

Punith Nayak

Done

Open in Gerrit

Related details

Attention is currently required from:

Alvin Ji
Michael Wilson

Submit Requirements:

Code-Coverage
Code-Owners
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 25, 2025, 2:01:59 PM6/25/25

to Punith Nayak, Adam Raine, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Michael Wilson and Punith Nayak

Alvin Ji added 6 comments

File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-panner.html

Line 4, Patchset 9 (Latest): <title> Test k-rate AudioParams of PannerNode </title>

Alvin Ji . unresolved

Ditto

Line 4, Patchset 9 (Latest): <title> Test k-rate AudioParams of PannerNode </title>

Alvin Ji . unresolved

Remove ` `.

Line 15, Patchset 9 (Latest): // If any element differs from the first by more than a negligible amount,

Alvin Ji . unresolved

let's wrap it at 80 columns

Line 38, Patchset 9 (Latest): // Defines the size of one audio processing block (render quantum) in frames.

Alvin Ji . unresolved

Ditto: 80 columns.

Line 129, Patchset 9 (Latest): `Panner ${param.name} k‑rate frames [${k}, ${k + slice.length - 1}]`);

Alvin Ji . unresolved

Ditto: 80 columns.

Line 151, Patchset 9 (Latest): listenerParams.forEach(p => {

Alvin Ji . unresolved

Let's use `param` instead of `p`.

Open in Gerrit

Related details

Attention is currently required from:

Michael Wilson
Punith Nayak

Submit Requirements:

Code-Coverage
Code-Owners
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

Punith Nayak (Gerrit)

unread,

Jun 25, 2025, 8:58:08 PM6/25/25

to Alvin Ji, Adam Raine, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Alvin Ji and Michael Wilson

Punith Nayak added 6 comments

File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-panner.html

Line 4, Patchset 9: <title> Test k-rate AudioParams of PannerNode </title>

Alvin Ji . resolved

Ditto

Punith Nayak

Done

Line 4, Patchset 9: <title> Test k-rate AudioParams of PannerNode </title>

Alvin Ji . resolved

Remove ` `.

Punith Nayak

Done

Line 15, Patchset 9: // If any element differs from the first by more than a negligible amount,

Alvin Ji . resolved

let's wrap it at 80 columns

Punith Nayak

Done

Line 38, Patchset 9: // Defines the size of one audio processing block (render quantum) in frames.

Alvin Ji . resolved

Ditto: 80 columns.

Punith Nayak

Done

Line 129, Patchset 9: `Panner ${param.name} k‑rate frames [${k}, ${k + slice.length - 1}]`);

Alvin Ji . resolved

Ditto: 80 columns.

Punith Nayak

Done

Line 151, Patchset 9: listenerParams.forEach(p => {

Alvin Ji . resolved

Let's use `param` instead of `p`.

Punith Nayak

Done

Open in Gerrit

Related details

Attention is currently required from:

Alvin Ji
Michael Wilson

Submit Requirements:

Code-Coverage
Code-Owners
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 26, 2025, 12:14:36 AM6/26/25

to Punith Nayak, Adam Raine, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Michael Wilson and Punith Nayak

Alvin Ji voted and added 2 comments

Votes added by Alvin Ji

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 10 (Latest):

Alvin Ji . resolved

LGTM

File-level comment, Patchset 10 (Latest):

Alvin Ji . resolved

LGTM

Open in Gerrit

Related details

Attention is currently required from:

Michael Wilson
Punith Nayak

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Adam Raine (Gerrit)

unread,

Jun 26, 2025, 1:15:43 PM6/26/25

to Punith Nayak, Alvin Ji, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Michael Wilson and Punith Nayak

Adam Raine voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Michael Wilson
Punith Nayak

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Adam Raine (Gerrit)

unread,

Jun 26, 2025, 1:17:04 PM6/26/25

to Punith Nayak, Alvin Ji, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Michael Wilson and Punith Nayak

Adam Raine added 1 comment

Commit Message

Line 13, Patchset 10 (Latest):Change-Id: I6004cd19223917566c43849cb969b9d0a8adb76d

Adam Raine . unresolved

```suggestion
Bug: 396477778
Change-Id: I6004cd19223917566c43849cb969b9d0a8adb76d
```

Open in Gerrit

Related details

Attention is currently required from:

Michael Wilson
Punith Nayak

Submit Requirements:

Code-Coverage
Code-Owners
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

Punith Nayak (Gerrit)

unread,

Jun 27, 2025, 6:05:18 PM6/27/25

to Adam Raine, Alvin Ji, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Michael Wilson

Punith Nayak voted and added 1 comment

Votes added by Punith Nayak

Commit-Queue

+2

1 comment

Commit Message

Line 13, Patchset 10:Change-Id: I6004cd19223917566c43849cb969b9d0a8adb76d

Adam Raine . resolved

```suggestion
Bug: 396477778
Change-Id: I6004cd19223917566c43849cb969b9d0a8adb76d
```

Punith Nayak

Fix applied.

Open in Gerrit

Related details

Attention is currently required from:

Michael Wilson

Submit Requirements:

Code-Coverage
Code-Owners
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 27, 2025, 6:35:23 PM6/27/25

to Punith Nayak, Adam Raine, Alvin Ji, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Chromium LUCI CQ submitted the change

Unreviewed changes

10 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

Change information

Commit message:

[webaudio] Migrate PannerNode and Listener k-rate tests to testharness.js

Replaces audit.js with testharness.js in k-rate automation tests for
PannerNode and AudioListener. Refactors assertions using testharness
primitives.

Bug: 396477778

Change-Id: I6004cd19223917566c43849cb969b9d0a8adb76d

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

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

Reviewed-by: Adam Raine <asr...@chromium.org>

Commit-Queue: Punith Nayak <punith...@chromium.org>

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

Files:

M third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-panner.html

Change size: L

Delta: 1 file changed, 186 insertions(+), 136 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Adam Raine, +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

Blink W3C Test Autoroller (Gerrit)

unread,

Jun 27, 2025, 7:50:50 PM6/27/25

to Chromium LUCI CQ, Punith Nayak, Adam Raine, Alvin Ji, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org

Message from Blink W3C Test Autoroller

The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/53456

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[webaudio] Migrate PannerNode and Listener k-rate tests to testharness.js [chromium/src : main]

Michael Wilson (Gerrit)

Michael Wilson added 1 comment

Related details

Punith Nayak (Gerrit)

Punith Nayak added 1 comment

Related details

Michael Wilson (Gerrit)

Michael Wilson added 6 comments

Related details

Punith Nayak (Gerrit)

Punith Nayak added 6 comments

Related details

Alvin Ji (Gerrit)

Alvin Ji added 3 comments

Related details

Punith Nayak (Gerrit)

Punith Nayak added 3 comments

Related details

Alvin Ji (Gerrit)

Alvin Ji added 1 comment

Related details

Punith Nayak (Gerrit)

Punith Nayak added 1 comment

Related details

Alvin Ji (Gerrit)

Alvin Ji added 6 comments

Related details

Punith Nayak (Gerrit)

Punith Nayak added 6 comments

Related details

Alvin Ji (Gerrit)

Alvin Ji voted and added 2 comments

Votes added by Alvin Ji

2 comments

Related details

Adam Raine (Gerrit)

Adam Raine voted Code-Review+1

Related details

Adam Raine (Gerrit)

Adam Raine added 1 comment

Related details

Punith Nayak (Gerrit)

Punith Nayak voted and added 1 comment

Votes added by Punith Nayak

1 comment

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Unreviewed changes

Change information

Blink W3C Test Autoroller (Gerrit)

Message from Blink W3C Test Autoroller

Related details