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

Reorganize(/cleanup) tests #533

Closed
jackh726 opened this issue Jun 17, 2020 · 0 comments · Fixed by #591
Closed

Reorganize(/cleanup) tests #533

jackh726 opened this issue Jun 17, 2020 · 0 comments · Fixed by #591
Labels
good first issue A good issue to start working on Chalk with

Comments

@jackh726
Copy link
Member

We've recently added a bunch of builtin types and traits, and a lot of tests associated with them. We should decide whether we're organizing tests by the trait the implement, or the type that implements.

Right now, there are some tests in test/builtin_impls, but much more in test/*. For example, there is test/builtin_impl/copy.rs which has only the test for tuples being copy. All the other Copy-related tests are in separate files. For example, fn_def_is_copy is in test/fn_def.rs.

I imagine it'll be easiest to just have each type be a separate file, so testing for Copy, Clone, WF, etc. would all be in one file. This would roughly line up with each row in http://rust-lang.github.io/chalk/book/clauses/well_known_traits.html being in a separate file.

I somewhat think that Unsize might be different. The tests are somewhat complicated, and I would feel better about being able to reason about the same logic across different types. Additionally, we can somewhat justify this since these tests are really testing the conversion of one type to another.

Somewhat related to this, it might be nice to go through and document why we have each test (as well as the individual goals in each test). Though, I'm not sure if that should be a part of this issue or a separate issue.

@jackh726 jackh726 added the good first issue A good issue to start working on Chalk with label Jun 17, 2020
@bors bors closed this as completed in #591 Aug 4, 2020
@bors bors closed this as completed in eed228c Aug 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue A good issue to start working on Chalk with
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant