Use NWBInspector when testing livescript tutorials

+tests/+fixtures/ResetGeneratedTypesFixture.m

@@ -0,0 +1,16 @@
+classdef ResetGeneratedTypesFixture < matlab.unittest.fixtures.Fixture
+    % ResetGeneratedTypesFixture - Fixture for resetting generated NWB classes.
+    %
+    %   ResetGeneratedTypesFixture clears all the generated classes for NWB
+    %   types from the matnwb folder. When the fixture is set up, all generated
+    %   class files for NWB types are deleted. When the fixture is torn down, 
+    %   generateCore is called to regenerate the classes for NWB types of the 
+    %   latest NWB version
+
+    methods
+        function setup(fixture)
+            fixture.addTeardown( @generateCore )
+            nwbClearGenerated()
+        end
+    end
+end

+tests/+sanity/GenerationTest.m

@@ -5,8 +5,14 @@
 
     methods (TestClassSetup)
         function setupClass(testCase)
-            rootPath = fullfile(fileparts(mfilename('fullpath')), '..', '..');
-            testCase.applyFixture(matlab.unittest.fixtures.PathFixture(rootPath));
+
+            import matlab.unittest.fixtures.PathFixture
+            import tests.fixtures.ResetGeneratedTypesFixture
+
+            rootPath = tests.util.getProjectDirectory();
+            testCase.applyFixture( PathFixture(rootPath) );
+
+            testCase.applyFixture( ResetGeneratedTypesFixture );
         end
     end
 

+tests/+system/PyNWBIOTest.m

@@ -34,7 +34,7 @@ function testInFromPyNWB(testCase)
 
     methods
         function [status, cmdout] = runPyTest(testCase, testName)
-            setenv('PYTHONPATH', fileparts(mfilename('fullpath')));
+            tests.util.addFolderToPythonPath( fileparts(mfilename('fullpath')) )
 
             envPath = fullfile('+tests', 'env.mat');
             if 2 == exist(envPath, 'file')

+tests/+unit/TutorialTest.m

@@ -3,17 +3,27 @@
 %
 %   This test will test most tutorial files (while skipping tutorials with 
 %   dependencies) If the tutorial creates an nwb file, the test will also try 
-%   to open this with pynwb.
-%   
-%   Note: 
-%       - Requires MATLAB XXXX to run py.* commands.
-%       - pynwb must be installed in the python environment returned by
-%         pyenv()
+%   to open this with pynwb and run nwbinspector on the file.
+
+%   Notes: 
+%   - Requires MATLAB 2019b or later to run py.* commands.
+%
+%   - pynwb must be installed in the python environment returned by pyenv()
+%
+%   - Running NWBInspector as a Python package within MATLAB on GitHub runners 
+%     currently encounters compatibility issues between the HDF5 library and 
+%     h5py. As a workaround in this test, the CLI interface is used to run 
+%     NWBInspector and the results are manually parsed. This approach is not 
+%     ideal, and hopefully can be improved upon.
 
     properties
         MatNwbDirectory
     end
 
+    properties (Constant)
+        NwbInspectorSeverityLevel = 1
+    end
+
     properties (TestParameter)
         % TutorialFile - A cell array where each cell is the name of a
         % tutorial file. testTutorial will run on each file individually
@@ -30,12 +40,22 @@
 
         % SkippedFiles - Name of exported nwb files to skip reading with pynwb
         SkippedFiles = {'testFileWithDataPipes.nwb'} % does not produce a valid nwb file
+
+        % PythonDependencies - Package dependencies for running pynwb tutorials
+        PythonDependencies = {'nwbinspector'}
+    end
+
+    properties (Access = private)
+        NWBInspectorMode = "python"
     end
 
     methods (TestClassSetup)
         function setupClass(testCase)
+
+            import tests.fixtures.ResetGeneratedTypesFixture
+
             % Get the root path of the matnwb repository
-            rootPath = getMatNwbRootDirectory();
+            rootPath = tests.util.getProjectDirectory();
             tutorialsFolder = fullfile(rootPath, 'tutorials');
 
             testCase.MatNwbDirectory = rootPath;
@@ -44,29 +64,16 @@ function setupClass(testCase)
             testCase.applyFixture(matlab.unittest.fixtures.PathFixture(rootPath));
             testCase.applyFixture(matlab.unittest.fixtures.PathFixture(tutorialsFolder));
 
-            % Note: The following seems to not be working on the azure pipeline
-            % Keep for reference
-
-            % % % Make sure pynwb is installed in MATLAB's Python Environment
-            % % args = py.list({py.sys.executable, "-m", "pip", "install", "pynwb"});
-            % % py.subprocess.check_call(args);
-            % % 
-            % % % Add pynwb to MATLAB's python environment path
-            % % pynwbPath = getenv('PYNWB_PATH');
-            % % if count(py.sys.path, pynwbPath) == 0
-            % %     insert(py.sys.path,int32(0),pynwbPath);
-            % % end
-
-            % % Alternative: Use python script for reading file with pynwb
-            setenv('PYTHONPATH', fileparts(mfilename('fullpath')));
-
-            nwbClearGenerated()
-        end
-    end
+            % Check if it is possible to call py.nwbinspector.* functions.
+            % When running these tests on Github Actions, calling 
+            % py.nwbinspector does not work, whereas the CLI can be used instead.
+            try 
+                py.nwbinspector.is_module_installed('nwbinspector');
+            catch
+                testCase.NWBInspectorMode = "CLI";
+            end
 
-    methods (TestClassTeardown)
-        function tearDownClass(testCase) %#ok<MANU>
-            %generateCore()
+            testCase.applyFixture( ResetGeneratedTypesFixture );
         end
     end
 
@@ -79,64 +86,177 @@ function setupMethod(testCase)
 
     methods (Test)
         function testTutorial(testCase, tutorialFile) %#ok<INUSD>
+            % Intentionally capturing output, in order for tests to cover
+            % code which overloads display methods for nwb types/objects.
             C = evalc( 'run(tutorialFile)' ); %#ok<NASGU>
-            testCase.testReadTutorialNwbFileWithPynwb()
+
+            testCase.readTutorialNwbFileWithPynwb()
+            testCase.inspectTutorialFileWithNwbInspector()
         end
     end
 
     methods 
-        function testReadTutorialNwbFileWithPynwb(testCase)
+        function readTutorialNwbFileWithPynwb(testCase)
 
             % Retrieve all files generated by tutorial
-            nwbListing = dir('*.nwb');
+            nwbFileNameList = testCase.listNwbFiles();
+            for nwbFilename = nwbFileNameList
+                try
+                    io = py.pynwb.NWBHDF5IO(nwbFilename);
+                    nwbObject = io.read();
+                    testCase.verifyNotEmpty(nwbObject, 'The NWB file should not be empty.');
+                    io.close()
+                catch ME
+                    error(ME.message)
+                end
+            end
+        end
+
+        function inspectTutorialFileWithNwbInspector(testCase)
+            % Retrieve all files generated by tutorial
+            nwbFileNameList = testCase.listNwbFiles();
+            for nwbFilename = nwbFileNameList
+                if testCase.NWBInspectorMode == "python"
+                    results = py.list(py.nwbinspector.inspect_nwbfile(nwbfile_path=nwbFilename));
+                    results = testCase.convertNwbInspectorResultsToStruct(results);
+                elseif testCase.NWBInspectorMode == "CLI"
+                    [~, m] = system(sprintf('nwbinspector %s --levels importance', nwbFilename));
+                    results = testCase.parseNWBInspectorTextOutput(m);
+                end
+
+                if isempty(results)
+                    return
+                end
+
+                results = testCase.filterNWBInspectorResults(results);
+                % T = struct2table(results); disp(T)
+
+                for j = 1:numel(results)
+                    testCase.verifyLessThan(results(j).importance, testCase.NwbInspectorSeverityLevel, ...
+                        sprintf('Message: %s\nLocation: %s\n File: %s\n', ...
+                        string(results(j).message), results(j).location, results(j).filepath))
+                end
+            end
+        end
+    end
 
-            for i = 1:numel(nwbListing)
-                nwbFilename = nwbListing(i).name;
-                if any(strcmp(nwbFilename, tests.unit.TutorialTest.SkippedFiles))
-                    continue
+    methods (Access = private)
+        function nwbFileNames = listNwbFiles(testCase)
+            nwbListing = dir('*.nwb');
+            nwbFileNames = string( {nwbListing.name} );
+            nwbFileNames = setdiff(nwbFileNames, testCase.SkippedFiles);
+            assert(isrow(nwbFileNames), 'Expected output to be a row vector')
+            if ~isscalar(nwbFileNames)
+                if iscolumn(nwbFileNames)
+                    nwbFileNames = transpose(nwbFileNames);
                 end
+            end
+        end
+    end
 
+    methods (Static)
+        function resultsOut = convertNwbInspectorResultsToStruct(resultsIn)
+
+            resultsOut = tests.unit.TutorialTest.getEmptyNwbInspectorResultStruct();
+
+            C = cell(resultsIn);
+            for i = 1:numel(C)
+                resultsOut(i).importance = double( py.getattr(C{i}.importance, 'value') );
+                resultsOut(i).severity = double( py.getattr(C{i}.severity, 'value') );
+
                 try
+                    resultsOut(i).location =  string(C{i}.location);
+                catch
+                    resultsOut(i).location = "N/A";
+                end
+
+                resultsOut(i).message = string(C{i}.message);
+                resultsOut(i).filepath = string(C{i}.file_path);
+                resultsOut(i).check_name = string(C{i}.check_function_name);
+            end
+        end
+
+        function resultsOut = parseNWBInspectorTextOutput(systemCommandOutput)
+            resultsOut = tests.unit.TutorialTest.getEmptyNwbInspectorResultStruct();
+
+            importanceLevels = containers.Map(...
+                ["BEST_PRACTICE_SUGGESTION", ...
+                "BEST_PRACTICE_VIOLATION", ...
+                "CRITICAL", ...
+                "PYNWB_VALIDATION", ...
+                "ERROR"], 0:4 );
+
+            lines = splitlines(systemCommandOutput);
+            count = 0;
+            for i = 1:numel(lines)
+                % Example line:
+                % '.0  Importance.BEST_PRACTICE_VIOLATION: behavior_tutorial.nwb - check_regular_timestamps - 'SpatialSeries' object at location '/processing/behavior/Position/SpatialSeries'
+                %                                        ^2                      ^1                         ^2                        ^ ^ ^ 3 
+                %      [-----------importance------------]  [------filepath------]  [------check_name------]                                                                           [-----------------location----------------]   
+                % Splitting and components is exemplified above. 
+
+                if ~isempty(regexp( lines{i}, '^\.\d{1}', 'once' ) )
+                    count = count+1;
+                    % Split line into separate components
+                    splitLine = strsplit(lines{i}, ':');
+                    splitLine = [...
+                        strsplit(splitLine{1}, ' '), ...
+                        strsplit(splitLine{2}, '-') ...
+                        ];
+
+                    resultsOut(count).importance = importanceLevels( extractAfter(splitLine{2}, 'Importance.') );
+                    resultsOut(count).filepath = string(strtrim( splitLine{3} ));
+                    resultsOut(count).check_name = string(strtrim(splitLine{4} ));
                     try
-                        io = py.pynwb.NWBHDF5IO(nwbListing(i).name);
-                        nwbObject = io.read();
-                        testCase.verifyNotEmpty(nwbObject, 'The NWB file should not be empty.');
-                        io.close()
-
-                    catch ME
-                        if strcmp(ME.identifier, 'MATLAB:undefinedVarOrClass') && ...
-                                contains(ME.message, 'py.pynwb.NWBHDF5IO')
-
-                            pythonExecutable = tests.util.getPythonPath();
-                            cmd = sprintf('"%s" -B -m read_nwbfile_with_pynwb %s',...
-                                            pythonExecutable, nwbFilename);
-
-                            status = system(cmd);
-                            if status ~= 0
-                                error('Failed to read NWB file "%s" using pynwb', nwbFilename)
-                            end
-                        else
-                            rethrow(ME)
-                        end
+                        locationInfo = strsplit(splitLine{end}, 'at location');
+                        resultsOut(count).location = string(strtrim(eval(locationInfo{2})));
+                    catch 
+                        resultsOut(count).location = 'N/A';
                     end
+                    resultsOut(count).message = string(strtrim(lines{i+1}));
+                end
+            end
+        end
 
-                catch ME
-                    error(ME.message)
-                    %testCase.verifyFail(sprintf('Failed to read file %s with error: %s', nwbListing(i).name, ME.message));
+        function emptyResults = getEmptyNwbInspectorResultStruct()
+            emptyResults = struct(...
+                'importance', {}, ...
+                'severity', {}, ...
+                'location', {}, ...
+                'filepath', {}, ...
+                'check_name', {}, ...
+                'ignore', {});
+        end
+
+        function resultsOut = filterNWBInspectorResults(resultsIn)
+            CHECK_IGNORE = [...
+                "check_image_series_external_file_valid", ...
+                "check_regular_timestamps"
+                ];
+
+            for i = 1:numel(resultsIn)
+                resultsIn(i).ignore = any(strcmp(CHECK_IGNORE, resultsIn(i).check_name));
+
+                % Special case to ignore
+                if resultsIn(i).location == "/acquisition/ExternalVideos" && ...
+                        resultsIn(i).check_name == "check_timestamps_match_first_dimension"
+                    resultsIn(i).ignore = true;
                 end
             end
+            resultsOut = resultsIn;
+            resultsOut([resultsOut.ignore]) = [];
         end
     end
 end
 
 function tutorialNames = listTutorialFiles()
 % listTutorialFiles - List names of all tutorial files (exclude skipped files)
-    rootPath = getMatNwbRootDirectory();
-    L = dir(fullfile(rootPath, 'tutorials'));
+    rootPath = tests.util.getProjectDirectory();
+    L = cat(1, ...
+        dir(fullfile(rootPath, 'tutorials', '*.mlx')), ...
+        dir(fullfile(rootPath, 'tutorials', '*.m')) ...
+        );
+
     L( [L.isdir] ) = []; % Ignore folders
     tutorialNames = setdiff({L.name}, tests.unit.TutorialTest.SkippedTutorials);
 end
-
-function folderPath = getMatNwbRootDirectory()
-    folderPath = fileparts(fileparts(fileparts(mfilename('fullpath'))));
-end

+tests/+unit/untypedSetTest.m

@@ -32,13 +32,15 @@ function testCreateSetFromNvPairsPlusFunctionHandle(testCase)
 end
 
 function testDisplayEmptyObject(testCase)
-    emptyUntypedSet = types.untyped.Set();
-    disp(emptyUntypedSet)
+    emptyUntypedSet = types.untyped.Set(); %#ok<NASGU>
+    C = evalc( 'disp(emptyUntypedSet)' );
+    testCase.verifyClass(C, 'char')
 end
 
 function testDisplayScalarObject(testCase)
-    scalarSet = types.untyped.Set('a',1)
-    disp(scalarSet)
+    scalarSet = types.untyped.Set('a', 1); %#ok<NASGU>
+    C = evalc( 'disp(scalarSet)' );
+    testCase.verifyClass(C, 'char')
 end
 
 function testGetSetSize(testCase)

+tests/+util/addFolderToPythonPath.m

@@ -0,0 +1,13 @@
+function addFolderToPythonPath(folderPath)
+    pythonPath = getenv('PYTHONPATH');
+    if isempty(pythonPath)
+        updatedPythonPath = folderPath;
+    else
+        if ~contains(pythonPath, folderPath)
+            updatedPythonPath = strjoin({pythonPath, folderPath}, pathsep);
+        else
+            return
+        end
+    end
+    setenv('PYTHONPATH', updatedPythonPath);
+end

+tests/+util/getProjectDirectory.m

@@ -0,0 +1,3 @@
+function projectDirectory = getProjectDirectory()
+    projectDirectory = fullfile(fileparts(mfilename('fullpath')), '..', '..');
+end

+tests/requirements.txt

@@ -1,2 +1,3 @@
 pynwb
-hdf5plugin
+hdf5plugin
+nwbinspector