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

Add render test suite #379

Closed
wants to merge 2 commits into from

Conversation

chris-laplante
Copy link
Collaborator

No description provided.

@chris-laplante
Copy link
Collaborator Author

@djc Could you please take a look at this when you get a chance? The basic_progress_bar test is not working as I would expect. I call inc(1) but it is not actually updating the progress bar. I think something might still be wrong with the rate limiting logic?

Cargo.toml Outdated Show resolved Hide resolved
@djc
Copy link
Collaborator

djc commented Feb 17, 2022

Yeah... I think we should rip out the draw limiting in the BarState and instead rely on the LeakyBucket stuff in the draw target.

@chris-laplante
Copy link
Collaborator Author

Yeah... I think we should rip out the draw limiting in the BarState and instead rely on the LeakyBucket stuff in the draw target.

Feel free to repurpose this PR if you want to add commits to do that. It might make sense given to develop those fixes in tandem with the render test suite. I can help flesh out the test suite (as originally intended) but I'm not sure I can help much with LeakyBucket though.

@djc
Copy link
Collaborator

djc commented Feb 17, 2022

Do you want to test? #380

@chris-laplante
Copy link
Collaborator Author

Do you want to test? #380

Will do!

@djc
Copy link
Collaborator

djc commented Feb 22, 2022

I added back the test case in 3930cfc after I reverted some of my draw limiting changes and merged the move into an integration test as 0c153b8. Thanks!

@djc djc closed this Feb 22, 2022
@chris-laplante chris-laplante deleted the cpl/test_suite branch July 28, 2022 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants