Fix crash in TimeBins report due to an incorrect key#4641
Fix crash in TimeBins report due to an incorrect key#4641Myoldmopar merged 4 commits intodevelopfrom
Conversation
…, Pivotal #84202708. Got rid of old code that was making reporting the error condition more complex than it needed to be. Changed error message slightly.
| @JasonGlazer @lgentile it has been 7 days since this pull request was last updated. |
2 similar comments
| @JasonGlazer @lgentile it has been 7 days since this pull request was last updated. |
| @JasonGlazer @lgentile it has been 7 days since this pull request was last updated. |
| @Myoldmopar Do I need to do anything else on this? It seems like the Linux CI tests failed because of some network issue not related to my changes. |
| @Myoldmopar @lgentile @mjwitte I am trying to figure out a simple unit test related to this checkin. Mike had previously suggested a unit test approach that relied on refactoring the getinput routine to separate out the input processing from the getinput and then test the input processing. He provided an example for a unit test he did in: https://github.com/NREL/EnergyPlus/pull/4590/files It is a lot of work on code unrelated to the checkin. Does anyone have a simpler approach to adding a unit test? |
| I didn't dig around, but is it possible to extract that section of the IF block into its own function that you call from there and then validate that that block behaves properly? If you pass in as arguments everything that you need to make the decisions, then you won't need to set up anything global. Just call the function and check the output under all possible conditions. |
| Added a unit test but is not really testing much. |
| @JasonGlazer @lgentile it has been 7 days since this pull request was last updated. |
| Yeah, I'm not sure about the preprocessor issues, but CI was having issues a couple weeks ago. I merged develop in and we'll see. |
Fixed crash related to TimeBins report with incorrect key.
Add missed unit test from PR #4641, fix new function signature for performance
Myoldmopar changed the title 3 participants
GitHub #4405, Pivotal #84202708. Got rid of old code that was making reporting the error condition more complex than it needed to be. Changed error message slightly.