I was reflecting on having the initialization error messages being swallowed on the refactoring.
That was my fault. I’m really sorry for introducing it. I tested for everything I could think of, but didn’t think to test that the exception actually propagated. (We never tested this before.) I tested that exceptions were thrown and that messages existed. Not that the content of the message was preserved.
Hopefully we all agree now that output (either through the screen or via text files or whatever) does matter. Not down to the character, but there are some things that are very important to how users interact with Stan + the interfaces. If someone asks that output is tested, please include it. (Hopefully it’s not just me that’s sensitive to this. And please ask me to test output too.)
If anyone feels strongly otherwise, feel free to bring up any objections and reasons for them. Or a better way to make sure we continue to have strong interfaces.
For (2), I think everyone agreed before that output’s important. We just don’t have a comprehensive spec against which to test and our unit tests are obviously not complete enough as exemplified by (1).
I think it would be great if we could add some regression testing for this kind of thing. The question is how to proactively put in more tests rather than relying on regression tests (technically making sure that bugs that get fixed once don’t re-occur; so now we should have a test that output gets printed, but what else might we have forgotten to test?).
It’s hard to enumerate all the reasons something could fail.
For this particular fix, I added unit tests to make sure the output includes the error message from the exception during initialization. When originally refactoring the code, I had put in tests that verified that the initialization stopped with the correct overall message. This issue shouldn’t come up again now that tests are in place.