CHG: MAssembly hardening and unit tests - #191
Conversation
|
The crash here is a MEGAlib one which just happens on Ubuntu again - I have to fix that first |
|
Fixed. Bug was in nuclearizer, not in MEGAlib. |
| for (int i = 1; i < argc; i++) { | ||
| Option = argv[i]; | ||
| if (Option == "-h" || Option == "--help" || Option == "?" || Option == "-?") { | ||
| if (Option == "--help" || Option == "-h" || Option == "-?" || Option == "?") { |
There was a problem hiding this comment.
Out of interest: why was the order of the Options changed? Is it good practice to have the written-out option first, so it's easier for people looking at the code which option is being processed by that code?
| return false; | ||
| } | ||
| int Verbosity = Value.ToInt(); | ||
| if (Verbosity < c_Quiet || Verbosity > c_Info) { |
There was a problem hiding this comment.
I've seen in megalib that there is also c_Chatty and c_Extreme.
I guess we are not allowing for those in nuclearizer, hence they are excluded?
What if someone wants a "chatty" megalib?
| @@ -197,77 +185,120 @@ bool MAssembly::ParseCommandLine(int argc, char** argv) | |||
| Usage<<" Perform a test run to see if nuclearizer can be started up correctly."<<endl; | |||
| Usage<<" -v --verbosity:"<<endl; | |||
| Usage<<" Verbosity: 0: Quiet, 1: Errors, 2: Warnings, 3: Info"<<endl; | |||
There was a problem hiding this comment.
Maybe for clarity (there's many colons here):
| Usage<<" Verbosity: 0: Quiet, 1: Errors, 2: Warnings, 3: Info"<<endl; | |
| Usage<<" Verbosity: 0 = Quiet, 1 = Errors, 2 = Warnings, 3 = Info"<<endl; |
?
| } else if (Option == "--test" || Option == "-t") { | ||
| // Parse later | ||
| } else if (Option == "--auto" || Option == "-a") { | ||
| // Parse later | ||
| } |
There was a problem hiding this comment.
Remove this? (I guess // Parse later refers to the // Now parse all high level options block later in this function?)
| MSupervisor* m_Supervisor; | ||
|
|
||
| //! The interrupt flag - the analysis will stop when this flag is set | ||
| bool m_Interrupt; |
There was a problem hiding this comment.
About m_Interrupt: is it correct that this was just not used before? I can only find m_Interrupt being used (but also defined) in the apps now.
| { | ||
| char Arg0[] = "UTNAssembly"; | ||
| char Arg1[] = "--verbosity"; | ||
| char Arg2[] = "4"; | ||
| vector<char*> Args = { Arg0, Arg1, Arg2 }; | ||
| Passed = EvaluateFalse("ParseCommandLine()", "out-of-range --verbosity argument", "ParseCommandLine() returns false when verbosity is outside 0..3", | ||
| ParseSilently(Assembly, Args)) && Passed; | ||
| } |
There was a problem hiding this comment.
See my comment above: this is intended that we do not want to support verbosity 4 and 5 in nuclearizer?
| { | ||
| char Arg0[] = "UTNAssembly"; | ||
| char Arg1[] = "-m"; | ||
| char Arg2[] = "1"; | ||
| vector<char*> Args = { Arg0, Arg1, Arg2 }; | ||
| MString Output; | ||
| ParseAndCapture(Assembly, Args, Output); | ||
| Passed = EvaluateTrue("ParseCommandLine()", "-m 1", "-m 1 enables multithreading", Output.Contains("Command-line parser: Using multithreading: yes")) && Passed; | ||
| } |
There was a problem hiding this comment.
Is there any way to confirm that MSupervisor::m_UseMultiThreading was actually set to true? Seems like m_UseMultiThreading is private, and there is no method to get it..
| vector<char*> Args = { Arg0, Arg1, Arg2 }; | ||
| MString Output; | ||
| ParseAndCapture(Assembly, Args, Output); | ||
| Passed = EvaluateTrue("ParseCommandLine()", "--multithreading 0", "--multithreading 0 disables multithreading", Output.Contains("Command-line parser: Using multithreading: no")) && Passed; |
There was a problem hiding this comment.
Same thing here: can we check if m_UseMultiThreading in MSupervisor is actually false?
| // A stray argument which is not an option is silently ignored | ||
| { | ||
| char Arg0[] = "UTNAssembly"; | ||
| char Arg1[] = "SomeFile.roa"; | ||
| vector<char*> Args = { Arg0, Arg1 }; | ||
| MString Output; | ||
| ParseAndCapture(Assembly, Args, Output); | ||
| Passed = EvaluateFalse("ParseCommandLine()", "stray argument", "An argument which is not an option does not trigger an unknown-option warning", Output.Contains("Unknown option")) && Passed; | ||
| } |
| if (MGlobal::Initialize("UTNAssembly", "Unit tests for MAssembly") == false) return 1; | ||
|
|
||
| // An accepted command line ends in MSupervisor::LaunchUI(): make sure no test can ever | ||
| // open a window, no matter which options a future test case passes |
There was a problem hiding this comment.
Maybe move this comment more towards the top of the file (or duplicate it at the top), so that it's more visible, in case people were to add more unit tests? I don't think that I would scroll to the very bottom of the file if I were to add more unit tests.. 😅
2 participants
Unit test, bug fixes, and hardening for MAssembly class