builder/tests/07-repeated-field.rs needs a fix to pass

[jecolon]

Hello. First things first, this is an excellent learning resource for Rust. If I was a recruiter, I would require passing this workshop before hiring. =) Now to the issue: In the builder project tests/07-repeated-field.rs , the test can't pass because the env field isn't optional but the test code doesn't set it before calling build(). A line like .env("FOO=1".to_owned()) just before the call to build (line 49) should fix this. It made it pass for me.

[dtolnay]

This is intentional, you call env n times to get a vector of n environment variables. We don't need to require that the caller provide >0 environment variables.

[jecolon]

But is the test not passing also intentional? To make it pass and satisfy your requirement, env should be optional then, right?

[dtolnay]

It doesn't need to be wrapped in Option because an empty vec already tells the user of Command that env was called 0 times. The user of Command doesn't need the field to be wrapped in Option to tell them that; I think the test case should pass as written, with the appropriate implementation of the macro.

[jecolon]

OK I see. Thanks for the insight!

[ririsoft]

@dtolnay I fall into the same issue. I totally follow you on this one, but I could not find a specification for it in the comments. Maybe this could be added to 06-optional-field with a comment and test case ?

[kziemianek]

In test 4 there is the requirement:

// Generate a build method to go from builder to original struct. // // This method should require that every one of the fields has been explicitly // set; it should return an error if a field is missing. The precise error type // is not important. Consider using Box, which you can construct // using the impl From for Box

env is a field and it's not set in test 7 so my implementation also fails. I see room for a small improvement. It could be explicitly specified that for vec we don't need to check this condition.

@dtolnay thanks for great learning resource.