FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Render charts only once in time scale tests by benmccann · Pull Request #6347 · chartjs/Chart.js · GitHub

Render charts only once in time scale tests - #6347

Merged
etimberg merged 1 commit into
chartjs:masterfrom
benmccann:duplicate-test-renders
Jun 25, 2019
Merged

Render charts only once in time scale tests#6347
etimberg merged 1 commit into
chartjs:masterfrom
benmccann:duplicate-test-renders

Conversation

Copy link
Copy Markdown
Contributor

In many of the time scale tests, we render the chart and then update it to some certain dimensions. This makes it very difficult to step through the code with a debugger because the first time we hit any line of code doesn't affect the ultimate state of the chart. It also makes the tests take longer to run than necessary.

This passes the dimensions into the creation function so that it's created with the correct dimensions from the beginning.

kurkle commented Jun 23, 2019

Copy link
Copy Markdown
Member

I tend to extract tests for debugging, so haven't had this issue.
That said, the only objection I can think of is maybe losing testing of 2nd update to scale with new dimensions, can you check existence / add that kind of test?

Copy link
Copy Markdown
Contributor Author

Thanks for taking a look. None of the tests I've changed are testing a 2nd update of scale.

nagix left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good to me

benmccann requested a review from kurkle June 25, 2019 16:14
etimberg merged commit d480e11 into chartjs:master Jun 25, 2019
nagix added this to the Version 2.9 milestone Jun 26, 2019
exwm pushed a commit to exwm/Chart.js that referenced this pull request Apr 30, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL