Organize and cleanup the use of utils in test/

[rok-cesnovar]

Description

There are some cleanup things that should to be done in the test/ folder:

• EXPECT_MATRIX_NEAR macro is used a lot but defined in every file separately. Example: Let value_of and value_of_rec return expressions #1872 (comment)
• gp_*_test.cpp files could replace the use of pull_msg with EXPECT_THROW_MSG
• util.hpp are scattered all over the subfolders, reorganize them in /prim, /rev ,...
• utils should be in stan::math::test or stan::test
• functions/functors/... that are defined in the global space should be put in a file specific namespace Example: https://github.com/stan-dev/math/blob/develop/test/unit/math/mix/fun/col_test.cpp
  f() there should be in the col_test namespace.
• expect_matrix_eq should be replaced by EXPECT_MATRIX_NEAR
• expect_std_vector_eq should be replaced by a macro
• clean up _works_with_other_functions tests that are redundant after we introduced the expression testing framework
• clean up the typedefs (AVEC, AVAR types, etc)

Current Version:

v3.2.0

[t4c1]

It would also be nice to replace functions, such as expect_matrix_eq or expect_rel_near with macros, so they could print the line number, where they were called if the assertion fails.

[syclik]

@rok-cesnovar, I saw the PR and wanted to comment on the overall approach. Just a bunch of questions... you’ve probably already thought of these:

• why macros instead of functions? (I’m hoping the answer is that it’s not possible to do with functions.)
• naming and friendliness to devs... these look very close in name to Google Test’s naming convention. Is there a reason we want to do this?
• how would a dev know that these are extensions past Google Test and are in our code?
• do we have a dev-friendly doc for these? I’m thinking along the lines of Google Test’s markdown files, but it could be anything else as long as we can find it.

To be clear, I’m not opposed to the naming convention. But it’s good to know how devs are going to find info if they think it’s in Google Test instead of our code.

[rok-cesnovar]

• why macros instead of functions? (I’m hoping the answer is that it’s not possible to do with functions.)

It is certainly possible to do this with functions. However, the error thrown with a macro is a lot more friendly. If there is a mismatch it will say that there was a mismatch inside a function named "expect_matrix_eq" and not that it was throw in for example multiply_test, line 207. It's true that you can probably guess easily where the error was if the tests only have a single call to expect_matrix_eq. But a lot of functions have more of them and its always nice if the error directly references the line.

Not a huge fan of macros otherwise, but this seems like a good use case.

• these look very close in name to Google Test’s naming convention

One reason I used the naming convention is because Stan Math already extends the Google Test naming convention. See EXPECT_THROW_MSG_WITH_COUNT and EXPECT_THROW_MSG in https://github.com/stan-dev/math/blob/develop/test/unit/util.hpp#L9 Those are there for 5+ years.

The other reason is that I like following the naming convention of Google Test. If you think that is confusing, I am open to changing it. I just find it easier if we follow a naming convention. My line of thinking is: "Oh I see these tests used EXPECT_MATRIX_EQ. So there must be a EXPECT_MATRIX_FLOAT_EQ.". I might be in the minority, so if these names are confusing if theymatch Google test, I am open to other suggestions.

There is also a function in the Math library that has probably been there forever that is named expect_matrix_eq(). And the problem there is that it's not actually checking exact equality, it just used float_eq. I know the difference is not huge, but still.

• how would a dev know that these are extensions past Google Test and are in our code
  do we have a dev-friendly doc for these? I’m thinking along the lines of Google Test’s markdown files, but it could be anything else as long as we can find it.

That is what I am hoping goes in the getting started dev doc, which needs to be updated: #1883

That and reading tests, which is probably one of the better ways of familiarizing with a new codebase. At least to me.

[syclik]

Thanks, that all makes sense! Especially the reasoning for using the naming convention. I think the trade-off there is sound and hopefully we can make it easy for others to follow along.

I'd prefer if we set up the doc with the PR. Or else it won't get done... that happens way too much and it's just technical debt that keeps accumulating if we don't doc it at the time. I'll post on the PR itself asking for the doc; I'd prefer something that's imperfect than wait for #1883. The content still needs to be written.