Created my first unit test ever. Is this the way?

[dRAT3]

So I created my first unit test. It seems like creating testable code needs more functions to work properly. Feels like I'm overengineering using so many small functions. And the test is more work then the function itself.

Anyway I digress. Is this a good unit test? And did I test the correct thing?

Function under test:

/// Takes the Settings values as string slices and returns
/// them as a String with toml formatting
pub fn create_conf_toml(
    db_url: &str,
    file_path: &str,
    jsonrpc_url: &str,
    jsonrpc_url_2: &str,
    latency_1: &u32,
    latency_2: &u32,
    scan_api: &str,
    mythx_api: &str,
) -> Result<String, Box<dyn std::error::Error>> {
    // Create struct and convert to toml string.
    let settings_struct = Settings {
        storage: Storage {
            db_url: db_url.to_string(),
            file_path: file_path.to_string(),
        },
        jsonrpc: JsonRpc {
            url_1: jsonrpc_url.to_string(),
            url_2: jsonrpc_url_2.to_string(),
            latency_1: *latency_1,
            latency_2: *latency_2,
        },
        scan: Scan {
            key: scan_api.to_string(),
        },
        mythx: MythX {
            key: mythx_api.to_string(),
        },
    };
    let toml = toml::to_string(&settings_struct).unwrap();

    Ok(toml)
}

Test:

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_create_toml_formatting_and_correct_values() {
        let toml = create_conf_toml(
            "dbtest",
            "filetest",
            "jsontest1",
            "jsontest2",
            &40,
            &41,
            "scantest",
            "mythtest",
        )
        .unwrap();

        let test_values = vec![
            "[storage]",
            "db_url = \"dbtest\"",
            "file_path = \"filetest\"",
            "",
            "[jsonrpc]",
            "url_1 = \"jsontest1\"",
            "url_2 = \"jsontest2\"",
            "latency_1 = 40",
            "latency_2 = 41",
            "",
            "[scan]",
            "key = \"scantest\"",
            "",
            "[mythx]",
            "key = \"mythtest\"",
        ];

        let mut test_values_iter = test_values.iter();
        let mut toml_iter = toml.lines();
        assert!(
            test_values_iter.all(|&x| Some(x) == toml_iter.next()),
            "One of the lines in the toml string doesn't match the expected output"
        );
    }
}

[tuxmain]

You can also test erroneous data that should produce an error.

In order to avoid updating the test each time a parameter is added to the function, you can test creating the conf file and then reading it with the read_conf_toml function (if you have one), and compare. (however it makes it harder to track bugs: does the problem come from read or write?)

I would have put an assert_eq! in the loop rather than a loop in an assert!, because this way it is easier to print which value produced the error.

[mbrubeck]

Does this function even need to exist? It seems like it would be clearer for the caller to create a Settings struct explicitly and then call a to_toml method on it.

And, since all this function actually does is call toml::to_string, do you really need to unit-test it, given that the code it runs is all in the toml crate that you are relying on?

Why does the function return Result, when it never returns Err?

Especially in languages like Rust where the type system prevents many classes of errors, I think it's okay not to unit test trivial functions like this. I find end-to-end testing much more valuable.

[dbr]

@dRAT3

It seems like creating testable code needs more functions to work properly

Writing testable code is certainly a challenge. It's usually possible to structure code so it's easy to test (while remaining easy to use outside of tests), without requiring "extra" code" just for the tests.. but I certainly find this usually requires a bit of experimenting and refactoring

Regarding your specific example, it seems like the main benefit of the test is to verify you don't have a typo like assigning the json_url value to the struct file_path field (which might happen through a copy-paste or refactoring mistake). Definitely a good thing to test, as bugs like that can be a pain to track down later.

On that line of thinking, the create_conf_toml method looks very "accident prone": it would be easy to accidentally mix up the "file_path" and "jsonrpc_url" arguments and it to look perfectly correct on first glance.

I would personally ditch the create_conf_toml method entirely, and instead have the test look something like:

#[derive(PartialEq, Eq)]
struct Settings {
    ...
}

#[test]
fn test_toml_roundtrip() {
    let settings_struct = Settings {
        storage: Storage {
            db_url: "Database URL!".into(),
            file_path: "/example/file/path.txt".into(),
        ...
    }

    let expected = r#"
[storage]
db_url = "Database URL!"
file_path = "/example/file/path.txt"
...
"#;

    // Check object can be serialized as expected
    let obj_as_string = toml::to_string(&settings_struct).unwrap();
    assert_eq!(obj_to_string, expected_str);

    // Check it can be converted back
    let back_as_obj = toml::from_string(&obj_as_string).unwrap();
    assert_eq!(&back_as_obj, &settings_struct);
}

This doesn't require any extra helper functions (I'm assuming the create_conf_toml is a helper for the tests), and should make it quite clear if fields get mixed up, and also checks a roundtrip (that the loaded objected is same as previous one)

[dRAT3]

Does this function even need to exist?

This function was first coupled together with a write to disk. Now they are seperate it existed to save space in my run_config() function which asks the config values from the terminal.

Would it be better to have one massive function that asks for the values and then writes them to disk? Or better to have two helper functions one to set-up the toml and one to write to disk? Or better one helper function just to write?

I think it's okay not to unit test trivial functions like this.

Yes I agree, seems like unit testing creates unnecessary decoupling/creating lot's of abstractions. Tbf I was just unit testing for the sake of learning unit testing. Since most of my functions have side effects/use filesystem this was the first thing I came up with. Do you use unit tests? If so how many and when?

Can I just skip unit testing altogether and just do end-to-end / integration testing? The whole thing seems like a waste of time ...

[Hyeonu]

The create_conf_toml function does 2 things.

1. Construct Settings type from given arguments.
2. Serialize it into the toml string.

And what makes the test hard and verbose is the second part, and I don't think you need to test this part at all. You may want to make functions doing only one thing well. Why don't you make this function returns the Settings?

[ExpHP]

@dRAT3

Can I just skip unit testing altogether and just do end-to-end / integration testing? The whole thing seems like a waste of time ...

Something about this mindset really bothers me, because it's hard for me to picture unit tests as a chore. To me, adding unit tests has always felt refreshing, like a piece of the cognitive burden has been lifted from my shoulders.

But I don't write unit tests for everything. Most of what I truly care about is already tested by integration tests. And you shouldn't write a test that has no possibility of ever being broken. So I only write unit tests where they provide me a clear benefit beyond the integration tests. I write them to reduce the scope of code I have to review when something breaks. I write them to alleviate concerns I have about maintainability. E.g. "sure, I considered this edge case now, but what if I change the function later and ruin it?" Or, in some cases writing them down early can help clarify my thoughts on what I expect a function to return.

I would suggest at the very least considering writing unit tests in the following scenarios, which is when a lot of my own unit tests get written:

1. Debugging. When I have a bug, I tend to I take a look at all of the components of my application that could be causing the bug, and start writing unit tests against each thing to narrow down the cause. In this scenario, I stand to gain something from every test I write, regardless of whether it initially succeeds or fails.
2. Preparation before a refactor. Let's say you have this big hairy ball of logic that you've not yet found a good way to break down. And it works, you think.... but now you have to add something to it, and it's ugly and scary and you don't want to touch it. Adding unit tests can give you reassurance that the function works as intended (or reveal that it does not!); and more importantly, it gives you the power to clean it up without fear of breaking things.

---

Since most of my functions have side effects/use filesystem this was the first thing I came up with.

Well I'm not sure what kind of project you're working on, but to me, any function that does anything filesystem-related at all is inherently "high-level" and not the sort of thing I would unit test (unless it is a reusable abstraction around the filesystem).

Even when I am not writing tests, I strive to make most of my code relatively easy to test, because I believe that this is one of the best principles you can follow that just leads to better code architecture overall. Basically, try to decouple things and make difficult logic reusable through traits like Iterator (to allow one function to produce things, and another function to process them), and preferring io::Read/io::Write (which are implemented by &[u8] and Vec<u8> respectively) over Files, and etc.