Skip to content

test(radio): add performance tests for mat-radio-button #19552

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

Merged
merged 6 commits into from
Jul 10, 2020

Conversation

wagnermaciel
Copy link
Contributor

@wagnermaciel wagnermaciel commented Jun 5, 2020

Overview

  • Test the render speed of 2 radio buttons
  • Test the render speed of 10 radio buttons
  • Test the click speed between 2 radio buttons

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 5, 2020
@wagnermaciel wagnermaciel marked this pull request as ready for review June 8, 2020 20:26
@wagnermaciel wagnermaciel requested a review from a team as a code owner June 8, 2020 20:26
await runBenchmark({
id: 'render-two-radio-buttons',
url: '',
ignoreBrowserSynchronization: true,
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity: Should we disable browser synchronization by default? We set it quite often and it feels unnecessary to repeat that everywhere? Surely can be a follow-up 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, should be a simple change 👍

@wagnermaciel wagnermaciel requested a review from devversion June 9, 2020 17:06
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM, just one nit suggestion for the naming of the clicks between test. We should track the default for synchronization somewhere. Could you create a JIRA or Github ticket? (can be super-small)

@wagnermaciel wagnermaciel requested a review from devversion June 9, 2020 17:25
@devversion devversion added lgtm target: patch This PR is targeted for the next patch release labels Jun 10, 2020
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM. Only one nit comment about the test strategy for changing radio buttons.

@wagnermaciel wagnermaciel added the action: merge The PR is ready for merge by the caretaker label Jul 7, 2020
@mmalerba mmalerba merged commit 1a5aaab into angular:master Jul 10, 2020
mmalerba pushed a commit that referenced this pull request Jul 10, 2020
* test(radio): add performance tests for mat-radio-button

* fixup! test(radio): add performance tests for mat-radio-button

* fixup! test(radio): add performance tests for mat-radio-button

* fixup! test(radio): add performance tests for mat-radio-button

* fixup! test(radio): add performance tests for mat-radio-button

* fixup! test(radio): add performance tests for mat-radio-button
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Aug 10, 2020
@wagnermaciel wagnermaciel deleted the radio-benchmark branch January 14, 2021 19:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants