imagej / imagej-scripting

ImageJ-specific applications of the SciJava script languages
Other
25 stars 15 forks source link

Add tests for several scripts #25

Closed bnorthan closed 7 years ago

bnorthan commented 7 years ago

Add tests for several scripts and tweak the scripts (if needed) so that the tests pass.

bnorthan commented 7 years ago

Hi @hadim I added tests for most of my scripts. I skipped a couple of them that had too many images popping up, and too much IJ1 stuff.

I found a few bugs a long the way. In the Simple Filtering groovy script I had to cast the input arrays to double by adding as double[], I am almost positive that previously, the framework converted it properly. Anything change with that @ctrueden ?? No biggy, it was easy to get working, thought these type of things may trip up beginners.

bnorthan commented 7 years ago

After these tests are merged, I think it would be a good time to cut a release. @hadim anything else we should do??

hadim commented 7 years ago

Awesome.

I found an error when compiling under NetBeans :

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running net.imagej.scripting.ImageJ2ScriptTest
log4j:WARN No appenders could be found for logger (org.bushe.swing.event.EventService).
log4j:WARN Please initialize the log4j system properly.
log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for more info.
[100L, 100L]
axis 0: type: X length: 100
axis 1: type: Y length: 100
axis 2: type: Z length: 100
size 212 intensity -6.21226415094
size 8971 intensity -74.9077025973
Traceback (most recent call last):
  File "/home/hadim/Documents/Hadrien/Google_Drive/Documents/Code/postdoc/ij/imagej-scripting/target/classes/script_templates/Tutorials/Find_Template.py", line 22, in <module>
    from net.imglib2.img.display.imagej import ImageJFunctions
ImportError: No module named display
Unmatched input: sigma2
Unmatched input: sigma1
axis d: type: X length: 400
axis d: type: Y length: 400
axis d: type: Channel length: 2
axis d: type: Z length: 25
Tests run: 17, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 19.036 sec <<< FAILURE! - in net.imagej.scripting.ImageJ2ScriptTest
testCreateAndConvolvePoints(net.imagej.scripting.ImageJ2ScriptTest)  Time elapsed: 0.107 sec  <<< ERROR!
java.lang.NullPointerException
    at net.imagej.scripting.ImageJ2ScriptTest.testCreateAndConvolvePoints(ImageJ2ScriptTest.java:270)

Results :

Tests in error: 
  ImageJ2ScriptTest.testCreateAndConvolvePoints:270 NullPointer

Tests run: 17, Failures: 0, Errors: 1, Skipped: 0

Also my initial idea was to have one Test class per folder to avoid having thousands of tests in a single file. So maybe you should move the scripts from the Tutorials folder inside a new class called TutorialsScriptTest for example. (if you have a better idea on how to organize those test classes, I take it.)

hadim commented 7 years ago

Beside that @ctrueden you can update scijava and cut a new release whenever you want once this PR is merged.

ctrueden commented 7 years ago

@bnorthan wrote:

I am almost positive that previously, the framework converted it properly. Anything change with that @ctrueden ?? No biggy, it was easy to get working, thought these type of things may trip up beginners.

I agree this is confusing and unfortunate, especially for beginners. I do not have time to track down if/how this may have changed (might be a change in how Groovy behaves, might be a change in an op implementation signature, might be a change of script language such as Jython to Groovy...). It does not work now because the built-in signature requires a double[] (or double[][]) and not a List, but in Groovy, [...] are List. I believe the SciJava converter framework already handles those sorts of conversions, so if you say ops.run("foo.bar", ...) instead of ops.foo().bar(...) it should work. We could of course add built-in List sigs etc. in the built-in type-safe methods for all these cases, but I do not want to put much time into doing that yet since I am already planning to restructure how/where the built-in methods are provided.

hadim commented 7 years ago

The error is due to a missing dependency in the pom.xml. Adding the following fix it :

<dependency>
            <groupId>net.imglib2</groupId>
            <artifactId>imglib2-ij</artifactId>
        </dependency>

But add a new error :

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running net.imagej.scripting.ImageJ2ScriptTest
log4j:WARN No appenders could be found for logger (org.bushe.swing.event.EventService).
log4j:WARN Please initialize the log4j system properly.
log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for more info.
[100L, 100L]
axis 0: type: X length: 100
axis 1: type: Y length: 100
axis 2: type: Z length: 100
size 212 intensity -6.21226415094
size 8971 intensity -74.9077025973
template inverse = ArrayImg [40x40]
final = PlanarImg [400x400]
Unmatched input: sigma2
Unmatched input: sigma1
axis d: type: X length: 400
axis d: type: Y length: 400
axis d: type: Channel length: 2
axis d: type: Z length: 25
null = ArrayImg [13x13x37]
Tests run: 17, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 16.187 sec <<< FAILURE! - in net.imagej.scripting.ImageJ2ScriptTest
testCreateAndConvolvePoints(net.imagej.scripting.ImageJ2ScriptTest)  Time elapsed: 0.062 sec  <<< ERROR!
java.lang.NullPointerException
    at net.imagej.scripting.ImageJ2ScriptTest.testCreateAndConvolvePoints(ImageJ2ScriptTest.java:270)

Results :

Tests in error: 
  ImageJ2ScriptTest.testCreateAndConvolvePoints:270 NullPointer

Tests run: 17, Failures: 0, Errors: 1, Skipped: 0
bnorthan commented 7 years ago

I can't seem to replicate the error with 'CreateAndConvolvePoints'. I might actually prefer to skip testing 'Find_Template' since it is displaying a bunch of images, the main point of it is to demonstrate how to display FFTs. When I get a chance (Monday probably) I will re-organize, as to have one test for each folder.

hadim commented 7 years ago

It's ok, I have moved some scripts. But I still have the bug saying that getClass().getResource("/script_templates/Tutorials/Create_And_Convolve_Points.py") is null... while I can find the file in the JAR (and also in the source).

I don't know what is hapenning but I guess it's system related if you say it compiles correctly on your system.

bnorthan commented 7 years ago

Thanks for dividing the test scripts up @hadim, I just pulled your changes, and the tests all pass on my Windows system. I will test on Linux later on and see if I can replicate your issue with Create_And_Convolve_Points.

ctrueden commented 7 years ago

:+1: All tests pass on my macOS system. LGTM

hadim commented 7 years ago

It looks like Jenkins has the same bug as me. So it's not system related at the end.

I'll try to look into that later.

bnorthan commented 7 years ago

It's because of a capitalization error when opening the script

File scriptFile = new File(
                getClass().getResource("/script_templates/Tutorials/Create_And_Convolve_Points.py").toURI());

The script name is 'Create_and_Convolve_Points', 'and' not 'And'. I seem to remember bad things happening if you rename a git controlled file with, same name, but different capitalization. So it might be best to just change the test. Which I already did and can check, if that sounds OK to you

hadim commented 7 years ago

Everything should be ok now.

hadim commented 7 years ago

@ctrueden Good to cut a release whenever you want for me.

ctrueden commented 7 years ago

:+1: Thanks guys. Will do!

ctrueden commented 7 years ago

Version 0.5.0 has been released, and pom-scijava has been updated on master. Will upload to the Java-8 update site later in the next batch of uploads.