Added proper error handling in InputParser.jl #60
Conversation
test/runtests.jl
Outdated
| println("Error! Skipping Tests!") | ||
| elseif id == Int(InputParser.SoilNumberErrorId) | ||
| println("Error: Invalid input file!") | ||
| println("\t>Line $(e.line): Soil layer number must be larger than previous one!") |
There was a problem hiding this comment.
I see essentially the same code over and over again in this change. Can you refactor the code so that you only have this once?
There was a problem hiding this comment.
@EmilSoleymani, I would like you to think about refactoring the test cases to remove the frequently repeated code. Also, I don't see any test cases for the new exception. I would think that Julia would let you assert that the proper exception was raised. A test case around that would be good.
There was a problem hiding this comment.
@EmilSoleymani, I don't see what I'm looking for in the test cases. I might be missing it, or you might need to add a few more test cases. I'd like test cases that tests that the exceptions are raised correctly. In Pyunit the test case would have a syntax like @AssertRaises(ExceptionName). I did a quick google and I found on a testing with Julia webpage test syntax like this: @test_throws exception expr. Do you use that (or something similar) somewhere and I missed it?
I never got to adding that. I made an issue about it (#64) and will be adding it shortly. |
|
Thank you for the clarification @EmilSoleymani. I read the issue incorrectly. You are saying what you are going to do, and for some reason I interpreted as something that you have already done. 😄 It doesn't hurt that we had a conversation to clarify anyway. 😄 |
|
I have added the test cases which confirm the correct errors are being thrown given various bad input files. This is achieved using the |
| end | ||
| Base.showerror(io::IO, e::ParsingError) = print(io, "Could not parse file.") | ||
|
|
||
| # FoundationError is for when foundationOption is not read as 1 or 2 |
There was a problem hiding this comment.
I know you are using the same file format as the original file, but once we get this working, we should update the file format to use a more meaningful string for the input. Remember what 1 or 2 means is asking too much of the user. Ideally, we'll eventually have the GUI interface which would let someone pick their foundation option with a drop down.
There was a problem hiding this comment.
Looks good @EmilSoleymani. Once you fix the minor typos I indicated, I'll approve the pull request.
Added Exception Handling in InputParser.jl
If there is anything wrong with the input file format, like missing info or data of the wrong type (i.e. a floating point number was entered instead of a whole number), corresponding errors are thrown with useful error messages as feedback. Also incorporated catching these errors and handling them as needed when calling them in
runtests.jl.