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

Fix test generation #89

Closed
wants to merge 6 commits into from

Conversation

alice-i-cecile
Copy link
Collaborator

Objective

Context

This failure was discovered in #46.

Feedback wanted

  1. Do we want to keep this strategy? Or should it be replaced with careful unit tests?
  2. Chrome's implementation of the spec is only one of several possible options; see Question: why layout values are rounded? #77. Its behavior isn't always desirable.
  3. This strategy won't work for other layout algorithms; see Support multiple layout algorithms #28.
  4. The generated code is massive, and makes refactoring remarkably painful. See Rename sprawl::node::Stretch -> sprawl::node::Sprawl #79.
  5. Any help with tokio would be appreciated; I've never touched this section of Rust before.

@alice-i-cecile alice-i-cecile added bug Something isn't working code quality Make the code cleaner or prettier. build system Make continuous integration do the tedious things labels Jun 8, 2022
@alice-i-cecile alice-i-cecile marked this pull request as draft June 8, 2022 18:16
@TimJentzsch
Copy link
Collaborator

Do we want to keep this strategy? Or should it be replaced with careful unit tests?

IMO it shouldn't be the main testing strategy, it's definitely a painful to set up (you need to install external tools) so we can't expect every contributor to do it. I think the main tests should be done with unit tests and we can keep something like this in CI to catch edge-cases if feasible.

@alice-i-cecile
Copy link
Collaborator Author

Closing in favor of #133. These generated tests are a disaster to maintain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working build system Make continuous integration do the tedious things code quality Make the code cleaner or prettier.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix cargo run --package gentest
2 participants