[SERVER-79573] add IWYU export pragmas to unittest.h Created: 01/Aug/23 Updated: 16/Oct/23 |
|
| Status: | Backlog |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Billy Donahue | Assignee: | Backlog - Service Architecture |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | techdebt | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Assigned Teams: |
Service Arch
|
||||||||
| Participants: | |||||||||
| Description |
|
I just noticed a test (itoa_test.cpp in this case) was modified from its include of #include "mongo/unittest/unittest.h" to now use the granular includes of the components of the unittest library: #include "mongo/unittest/assert.h" This is a downgrade, as the unittest.h header is designed as an umbrella header for the API. The unit tests affected by this migration should have their includes of unittest/unittest.h restored. There's an IWYU pragma that can be added to unittest.h to prevent future IWYU passes from doing this again. We'd make unittest/unittest.h look like this:
|
| Comments |
| Comment by Billy Donahue [ 15/Aug/23 ] |
|
Optionally undo the IWYU changes that broke up the includes of the umbrella header. |
| Comment by Steve Gross [ 10/Aug/23 ] |
|
billy.donahue@mongodb.com SGTM; I see the issue is assigned to service arch. |
| Comment by Steve Gross [ 10/Aug/23 ] |
|
Per conversation w/ billy.donahue@mongodb.com :
|
| Comment by Jason Chan [ 09/Aug/23 ] |
|
steve.gross@mongodb.com, do we have an idea how real/significant the build penalty would be? Since Service Arch owns the unit test infrastructure, this decision directly impacts how that component is maintained moving forward. |
| Comment by Billy Donahue [ 09/Aug/23 ] |
|
Sure. Let's have that talk. |
| Comment by Steve Gross [ 08/Aug/23 ] |
|
billy.donahue@mongodb.com You make an interesting point. The use of "umbrella" includes is an interesting technique, and can be useful in some cases. The tradeoff, however, is possible inefficient/slower builds (hence the IWYU refactor to eliminate umbrella includes). It seems like a broader engineering discussion to have, however; FWIW, my hot take is that umbrellas aren't worth the build penalty. With that said, let's chat more about this; maybe start a slack thread to discuss in more detail? |