Add a proper command line interface to EnergyPlus #4544
Conversation
…dLineInterface with ShowFatalError
…L/EnergyPlusTeam into 1277852-CommandLineInterface
|
@Myoldmopar @kbenne @nealkruis Regarding the running of the preprocessors, it seems we keep wavering in the definition of what the default CLI usage should be - does it duplicate the existing scripts or not. Seems that an argument should be required to turn off the preprocessing steps rather than activate. |
|
I fixed the problem with the unit test. @mjwitte, the CLI usage should provide a posix-like interface to EnergyPlus. NREL has decided that it is best for the CLI to follow the executable behavior by default. The existing shell scripts will still function the same way, and the script behavior can be replicated through CLI arguments. |
|
Hmmm, I do not know what is going on with the Ubuntu build. Those integration tests are running fine for my debug build locally. And they run on all the other platforms. I'll pull develop in to get rid of the regression from the heat pump water heater commit, and maybe it will have just been an anomaly. |
Currently only removing the idd and idf file paths, could remove more; Also leaving the paths in for **error** messages; Could add these back in later if desired
|
OK, with this last commit, I removed the full file paths for the idd and idf when it is simply reporting that it is starting and finishing reading them. I didn't remove the file paths for the erroneous messages. I don't have a strong stand on this at all, but just flushing out full paths seems like the wrong approach unless the user needs to know (error condition). Also, I am not seeing the issue with the unit test that is showing up on CI; all the E+ unit tests run successfully. @kbenne @nealkruis could one of you confirm whether or not the unit tests are failing on your machines? |
|
@Myoldmopar I fixed the unit test here. |
|
Yep. I'm on the new one. This must be a recent merge from develop. |
|
Is it reasonable that all these files be given defaults so that we don't have to manually do this when writing unit tests? |
|
I think so. I'll go ahead and do that. |
|
@nealkruis So, things are still improving, but I noticed something. A new error is showing up because ExpandObjects can't find the idd file when it tries to run. See here. This should be remedied somehow. Next, it looks like the coverage warning will be triggered for integration test coverage. I suppose it is possible that once the above issue is cleaned up, it will hit those last few lines of code and bump up to the limit. But in case it isn't, here are my thoughts: @lgentile @kbenne The CLI is being exercised pretty heavily as a core part of our testing framework. It doesn't really need to be accompanied with a specific integration test. I'm not sure if the right answer here, but here are some options:
At the moment, I am more interested in getting the CLI working really well and merged, and less interested in the coverage warning. I can concoct an integration test that will bump up the coverage at any time. Maybe I'll just do that now and this will be moot. |
|
Nevermind on the above comment; I was apparently looking at the unit test coverage. |
This should add more code coverage and get this branch in good terms there; This example file had a height varying site object; but I dont think its used on DDs only; Adding the weather file sizing period should hit many lines of code in DataEnvironment
|
Oh, and then whatever is going on with the Windows cli-arg test. Which...since the CLI is being used directly in with the other integration tests, is probably not necessary anyway, and should be removed. |
|
I think I fixed the preprocessor related tests and the unit tests. Waiting on the CI for confirmation. What do you mean by "Windows cli-arg"? This? |
|
@kbenne @lgentile @nealkruis This can be merged at will once CI reports that it doesn't have any beef with beef3d5. |
Myoldmopar
changed the title
@kbenne @Myoldmopar This is the Command Line Interface pull. Can one of you review? I've tested on Ubuntu, OS X, and Windows 7.