Updating IDD and error handling for GHX:Vertical

[mitchute]

Addresses #5203.

Now catches wrong wrong values in the Num G-Func field and catches wrong number of inputs.

[Myoldmopar]

So, @mmAtTs. Unit test. I can imagine this wouldn't be too hard if you use the EnergyPlusFixture. You could, at a minimum, put invalid idf snippet that has the bad inputs we were encountering, and expect a failed process_idf or whatever the function is called.

[mitchute]

@Myoldmopar yeah, that should be easy enough. I will try to push something up tonight.

[EnergyArchmage]

I reviewed these changes, all seems good to go to me.

[Myoldmopar]

@mmAtTs Could you pull develop back into this branch one more time?

[mitchute]

@Myoldmopar I just pushed it up here. I'm not sure why it is not showing up in this PR though.

I got hung up on the unit tests, but once that is resolved this should be ready.

[Myoldmopar]

OK, so it looks like the unit test works. I think you might want to say EXPECT_TRUE though since you are checking an erroneous condition.

[Myoldmopar]

@mmAtTs @mbadams5 Once #5259 is merged in, maybe you guys could coordinate to get this unit test working with the new structure so the dashboard will be all green again and happy and 😀.

[mitchute]

@mbadams5 A couple of unit testing questions for you:

For these unit tests I am passing bad idf objects in here and here and trying to test the error handling here. I keep getting stuck because the out of range idd error handling here is stopping it. I just pulled develop in and have tried all combinations of ASSERT/EXPECT TRUE/FALSE. Do you have any suggestions on what I should be using around the process_idf so I can continue on to test that block? Also, I did not see any examples in unit tests of where were are checking for a fatal error. Is it possible to "EXPECT_DEATH" for something like that, or do I need to work around it some other way?

[mbadams5]

@mmAtTs I added new functionality to suppress the assertions within process_idf() for this use case. Normally these assertions shouldn't be suppressed but in this case you know the IDF is bad. So pass false after your idf_snippet and it should pass now.

[mitchute]

@mbadams5 thanks, that works. As far as expecting a fatal error, is there a way to do that? If not, I will probably need to do a little code work to make the errorsFound flag available.

[mbadams5]

@mmAtTs What do you mean "expecting a fatal error"? Like the function you are calling will call ShowFatalError or whatever it is called? Basically, if the function throws a Fatal Error then you need to wrap it in EXPECT_DEATH test because it will kill the whole unit test framework otherwise. Just be aware that you can only test that a function exits, aborts, or throws an exception when you use EXPECT_DEATH, nothing else.

If you are talking about something else then I need some more information.

[mitchute]

@mbadams5 yes, they eventually call ShowFatalError here.

So is EXPECT_DEATH( GetGroundHeatExchangerInput(), "" ); something along the lines of what I'm looking for? Or, EXPECT_EXIT( GetGroundHeatExchangerInput(), ::testing::ExitedWithCode( 1 ), "" );?

[mitchute]

@Myoldmopar ready for review.

[mbadams5]

@mmAtTs EXPECT_DEATH( GetGroundHeatExchangerInput(), "" ); is the same as EXPECT_EXIT( GetGroundHeatExchangerInput(), ::testing::ExitedWithCode( 1 ), "" ); and in fact more. EXPECT_DEATH checks for any exit that is not EXIT_SUCCESS, which is usually 0 but is platform dependent.

Also you can still wrap the process_idf() in EXPECT_TRUE() since it will return true if there is a failure. It will just not assert within the function if you pass false.

[mitchute]

@mbadams5 thanks for the information. I added EXPECT_TRUE around process_idf.

[Myoldmopar]

Hmmm, not sure why the other CI boxes decided not to work on 1c8e038. Maybe because of the underlying CI robustness improvements that were made? Anyway, just to be super careful, @mmAtTs, can you merge develop -- one more time. The changes look good, this will merge if CI is happy.