Guidelines for testing in taffy

[Weibye]

Objective

I'm proposing we flesh out and decide on some overall guidelines for writing tests, and where to place them, mostly just to ensure we produce the same kind of output and maintain readability though the project.

Context

1. While I have dabbled in it, I'm new to large-scale testing of projects.
2. I'm generally new to rust so I don't yet know the idiomatic ways of testing in rust.
3. In order to test private / internal behavior, unit-tests need to be in the same module (which means same file?)
4. This leads to very long files, which, coming from other languages, is unusual. However this seems both normal and expected in rust(?).

Questions

• Where should the unit-tests of private / internal behavior reside?
  • Alternative: Should we not care about testing internal behavior?
• Where should the unit-tests of public / external behaviour reside?
• Where should integration tests reside?
  • Here I'm pretty sure the answer is "in the ./tests/ folder"
• How focused should each test be?
  • Should each test only contain one concept, or multiple?

[Weibye]

Proposed guidelines

1. All tests should be wrapped in a module called tests, to ensure they are not compiled unless testing

#[cfg(test)]
mod tests {
    // Place tests here
}

2. For unit-testing this should be placed within the file of the definition they are testing
   1. If file becomes significantly long, tests should be split out into its own file under the same module

// file: ./src/my_struct.rs
struct MyStruct;

impl MyStruct {
    fn some_method() { .. }
}

#[cfg(test)]
mod tests {
    #[test]
    fn test_of_some_method() { .. }
}

4. For integration tests this should be placed within the ./tests folder

// file: ./tests/my_integration_test.rs
#[cfg(test)]
mod tests {
    #[test]
    fn integration_test_one() { .. }
    #[test]
    fn integration_test_two() { .. }
}

5. Each test should have a clear intent
   1. It should be evident what is being tested (naming, code, comments)
   2. When this test fails, it should be easy to understand why

I think the right place to put this would be in the CONTRIBUTING.md

[alice-i-cecile]

Some quick answers to your questions :) Excellent work though, once we've reached a bit of consensus can you open up a PR to our CONTRIBUTING.md to document this in a stable fashion?

In order to test private / internal behavior, unit-tests need to be in the same module (which means same file?)

Yep, this is the motivation for that convention. However, modules can be multiple files! The easiest / clearest way to do this is to create a file folder, e.g. src/node, split the code into sensible submodules as separate files within that folder, and then create a file called either mod.rs or node.rs (in this specific case) in that folder that imports all of the submodules. I prefer the latter style personally, but there's no functional difference.

This mod.rs file can be used to store some or all of the unit tests for the module, or we can store them in a dedicated module level tests file.

Where should the unit-tests of private / internal behavior reside?

So, this is mostly a matter of style. For this code base, I think that I'd like to say "in a node::tests module. This can either be in its own file if we have a full folder, or on the single file if it hasn't grown to that size.

Doc tests are just bonus, but should probably be preferred for really simple stuff.

Where should the unit-tests of public / external behaviour reside?

Same place as the other unit tests; whether or not something is public isn't important enough to draw this distinction IMO.

How focused should each test be? Should each test only contain one concept, or multiple?

I'd say "one clear purpose and concept". Some projects are stricter, and want "one assert per test", but I tend to find that leads to a lot of boilerplate as you set stuff up repeatedly.

The main goal here is if our test suite is failing, we should be able to look to the failing unit tests and be able to rapidly pinpoint what we broke. In the case of taffy in particular, this means that we're going to want a unit test for pretty much every statement made in the specs that we're following, since the logic is so arbitrary subtle and precisely specified.

[Weibye]

I've updated my commend to adjust with your insight. Will create PR then we can discuss the phrasing there if needed :)

[mcrvaz]

Nice, this seems like a good direction!
I've got some additional questions about this too.

• Currently there are some test cases using rstest. When is this preferred to simple rust tests? Should we use it at all?

• I was trying to come up with some tests for our Taffy struct, but it's very dependent on Forest. So, I'm also new to rust, but I wouldn't exactly call this a unit test since it interacts with other components. It also would probably be unecessarily hard to test, since you would need to known how this Forest structure works in detail so you can set up the test correctly. Anyway, the question is: should we be using some mock framework for this? Or should we just skip unit tests in this case?

[mcrvaz]

Yeah, this is one of my main concerns. If we unit test those things and they change completely later, then the unit tests would be kind of in vain, maybe not so if the dependencies were mocked.

But I guess I was too specific here, this question was supposed to sound more like: should we use mocking frameworks at all? Should we have guidelines for it?

About rstest, it can (or should?) be used if it makes the test simpler, right? Should this be included in the guidelines?

[Weibye]

About rstest, it can (or should?) be used if it makes the test simpler, right? Should this be included in the guidelines?

Yes! We should add a note about it, we can add it in the "make tests readable" point

[Weibye]

I've added a note about rstest, let me know if it can be improved in any way :)

[Weibye]

Also, as a note: This is an example of how not to use rstest It shows no intent or purpose.

Seeing that I wrote it, I'm working on cleaning this up 😅

[alice-i-cecile]

should we use mocking frameworks at all? Should we have guidelines for it?

I think we should try to avoid this for now because the library is relatively immature and small. If a concrete need arises, we can discuss it there. The added context really helps make these decisions IME.