NeurodataWithoutBorders / matnwb

A Matlab interface for reading and writing NWB files
BSD 2-Clause "Simplified" License
50 stars 32 forks source link

Tests are failing but CI is passing #255

Closed acampbel closed 3 years ago

acampbel commented 3 years ago

Hi,

I just noticed that some tests are failing, for example here:

https://dev.azure.com/NeurodataWithoutBorders/matnwb/_build/results?buildId=1340&view=logs&j=12f1170f-54f2-53f3-20dd-22fc7dff55f9&t=5c7b8cce-d95f-56ce-c8fa-472b8c0934b1&l=19

but the job is passing. IS this intended. Couple thoughts:

The run MATLAB command line doesn't need its own try-catch. For example this currently in the azure-pipelines file:

"try, results = nwbtest; if isempty(results), exit(1); end; catch err, disp(err.message); exit(1); end"

Can be replaced by this:

"results = assertSuccess(nwbtest); assert(~isempty(results), 'No tests ran'); "

The build will fail if the command errors, but the command won't error if a test fails unless you add assertSuccess.

Also, you may consider using the RunMATLABTests task, which also generates Junit and coverage artifacts just like you are doing manually, but may be easier and less error prone.

Hope that helps! Andy

rly commented 3 years ago

Great ideas! Thanks. We'll give this a try.

rly commented 3 years ago

Thanks @acampbel . We implemented the change and CI now fails correctly. We will resolve the test errors in a separate PR.