#583 xASL_test_IntegrationTesting: Initial implementation #606
Conversation
jan-petr
left a comment
There was a problem hiding this comment.
I think we are nearly there. I agree with your edits. Those weird calls of ExploreASL, we have tested previously. The main thing that was missing is that during the Flavors testing, we were calling directly the ExloreASL submodules and functions like xASL_module_Import or xASL_bids_BIDS2Legacy. This did the job, but wasn't testing if the master script is working. I now replaced all the calls with calls to the master script ExploreASL. All should work and should be better for testing.
One small thing is missing - it now initializes everything again upon each call of ExploreASL - can we do something about it? Probably not and we have to live with it, right? This would be also OK - more initializations are called because we want every call to be an independent call of ExploreASL, so that's the price we pay for this.
Once we solve this minor thing and once you check if all works (I have tested 1 flavor), then I will approve and I will merge. Note that I did some small restructuring of function names etc.
If we want the tests to be more independent, we can add a small script to remove the Matlab paths in between. Similar to the call in Henks old test script: % Remove ExploreASL paths
warning('off','MATLAB:rmpath:DirNotFound');
rmpath(genpath(x.MyPath));
warning('on','MATLAB:rmpath:DirNotFound');
|
Yes - good idea. Can you please add that? |
It's a bit difficult, since we have to make sure that we run it in the correct code section and we're still able to reinitialize etc. The script to remove the paths (920fc28) is pretty simple and I found two code sections where we can easily add this (7ac0165). |
|
Looks good, just don't store these in an |
HenkMutsaerts
left a comment
There was a problem hiding this comment.
Very nice work! Though I wonder if not many of the things that are now stated explicitly in the testing scripts should be stated inside the actual ExploreASL scripts, and the testing iterates them, so simplification. E.g., the logging should be part of ExploreASL as well right, why only logging when testing and not when a user runs the scripts?
I would make the actual running and testing as equal as possible, so debugging becomes the same if a report comes from our internal testing or from the user.
Logging is part of ExploreASL already. It can be still improved though. Check out The only thing that the integration testing script does now, is to combine all of the logs from the different flavors into an xlsx and a tsv file. IMHO we don't need this every time somebody runs ExploreASL in general. |
OK nice! |
HenkMutsaerts
left a comment
There was a problem hiding this comment.
Few minor issues but otherwise happy :)
f668525 to
43f5305
Compare
43f5305 to
99c4720
Compare
Linked issue
Check out issue #583
How to test
testConfig.jsonin your ExploreASL/Testing directory[TestResults, CheckResults] = xASL_test_IntegrationTesting;Comments
I know this probably doesn't fulfill all requirements yet, but I thought I could show you my initial implementation and how I want to extract the errors/warning in the future. The x struct includes a logging field now (implemented in #588). This includes both errors from the import and from the processing workflow. Those should be written into a
log.xlsxfile. It doesn't catch the various warnings and stuff right now, but the major things are nicely captured.