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

Playwright Port Performance Tests #384

Merged
merged 13 commits into from
Jul 31, 2024
Merged

Playwright Port Performance Tests #384

merged 13 commits into from
Jul 31, 2024

Conversation

terryzfeng
Copy link
Collaborator

@terryzfeng terryzfeng commented Jul 20, 2024

Porting Performance Tests from https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/perf_tests/webaudio/
to Web Audio Test Suite

  • Gain Test
  • Panner Test
  • Timeline Insert Event
  • Audio Buffer Source Node
  • Audio Worklet Node
  • Biquad FIlter Node
  • Dynamics Compressor Node Knee
  • Dynamics Compressor Node Post Knee
  • Dynamics Compressor Node Pre Knee

@Kizjkre Kizjkre marked this pull request as draft July 20, 2024 20:51
@mjwilson-google mjwilson-google added the gsoc24 Google Summer of Code 2024 label Jul 22, 2024
@terryzfeng terryzfeng changed the title [draft] Playwright Port Performance Tests Playwright Port Performance Tests Jul 24, 2024
@terryzfeng terryzfeng marked this pull request as ready for review July 24, 2024 01:22
Copy link
Collaborator

@Kizjkre Kizjkre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small changes

Copy link
Member

@hoch hoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks great! Hope we can tidy this further.

@terryzfeng terryzfeng requested a review from hoch July 26, 2024 00:49
Copy link
Member

@hoch hoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some directional comments:

  • If the comment explains "what's being done", probably it's better to revise the code so it is more readable and remove the comment.
  • Anything empirical/arbitrary (e.g. accuracy number) should have a comment on why. If it's unclear, we can always "this is arbitrary".
  • Assume that someone else will touch the code later. The code that is easier to read is better than the code being cryptic and clever!
  • Try to avoid general variable names (e.g. bufferData, float32buffer).

Thanks for your patience, and I think we are almost there!

@Kizjkre Kizjkre requested a review from hoch July 30, 2024 22:12
Copy link
Member

@hoch hoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nits

@Kizjkre Kizjkre requested a review from hoch July 30, 2024 23:05
Copy link
Member

@hoch hoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Let's get this out!

@hoch
Copy link
Member

hoch commented Jul 31, 2024

Thanks for your patience, @terryzfeng and @Kizjkre!

@hoch hoch merged commit 731fd47 into main Jul 31, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gsoc24 Google Summer of Code 2024
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants