New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test failures during "make check" stage of build #33

Closed
apjanke opened this Issue Jun 25, 2018 · 12 comments

Comments

2 participants
@apjanke
Contributor

apjanke commented Jun 25, 2018

Some tests are failing or producing error messages during the "make check" stage of the "brew install" that aren't failing when I run them in the installed app.

  libinterp/corefcn/graphics.cc-tst ........................... PASS     46/47  
                                                                  FAIL    1
                                                (missing feature) SKIP    3
...
  plot/util/copyobj.m .........................................  ./base/gsicc_manage.c:1172: gsicc_open_search(): Could not find default_rgb.icc 
| ./base/gsicc_manage.c:1838: gsicc_set_device_profile(): cannot find device profile
 PASS      1/2   
                                                                  FAIL    1
...
  plot/util/hgsave.m ..........................................  ./base/gsicc_manage.c:1172: gsicc_open_search(): Could not find default_rgb.icc 
| ./base/gsicc_manage.c:1838: gsicc_set_device_profile(): cannot find device profile
 PASS      3/4   
                                                                  FAIL    1
...

  publish.tst .................................................  ./base/gsicc_manage.c:1172: gsicc_open_search(): Could not find default_rgb.icc 
| ./base/gsicc_manage.c:1838: gsicc_set_device_profile(): cannot find device profile
  ./base/gsicc_manage.c:1172: gsicc_open_search(): Could not find default_rgb.icc 
| ./base/gsicc_manage.c:1838: gsicc_set_device_profile(): cannot find device profile
  ./base/gsicc_manage.c:1172: gsicc_open_search(): Could not find default_rgb.icc 
| ./base/gsicc_manage.c:1838: gsicc_set_device_profile(): cannot find device profile
  ./base/gsicc_manage.c:1172: gsicc_open_search(): Could not find default_rgb.icc 
| ./base/gsicc_manage.c:1838: gsicc_set_device_profile(): cannot find device profile
  ./base/gsicc_manage.c:1172: gsicc_open_search(): Could not find default_rgb.icc 
| ./base/gsicc_manage.c:1838: gsicc_set_device_profile(): cannot find device profile
  ./base/gsicc_manage.c:1172: gsicc_open_search(): Could not find default_rgb.icc 
| ./base/gsicc_manage.c:1838: gsicc_set_device_profile(): cannot find device profile
  ./base/gsicc_manage.c:1172: gsicc_open_search(): Could not find default_rgb.icc 
| ./base/gsicc_manage.c:1838: gsicc_set_device_profile(): cannot find device profile
 PASS      2/2   
...

Summary:

  PASS                            14935
  FAIL                                3

make-check.log

@apjanke apjanke added the bug label Jun 25, 2018

@apjanke apjanke self-assigned this Jun 25, 2018

@apjanke apjanke added this to Needs triage in Octave.app via automation Jun 25, 2018

@apjanke apjanke added this to the 4.4.0 milestone Jun 25, 2018

@apjanke apjanke moved this from Needs triage to High priority in Octave.app Jun 25, 2018

@schoeps

This comment has been minimized.

Show comment
Hide comment
@schoeps

schoeps Jun 25, 2018

Contributor

I think this is a consequence of our non-standard ghostscript folder. This is fixed in the installed app by

return "export GS_OPTIONS=\\"-sICCProfilesDir=$gs_share/$gs_ver/iccprofiles/ -sGenericResourceDir=$gs_share/$gs_ver/Resource/ -sFontResourceDir=$gs_share/$gs_ver/Resource/Font\\";"

We could export GS_OPTIONS in bundle_octave

Contributor

schoeps commented Jun 25, 2018

I think this is a consequence of our non-standard ghostscript folder. This is fixed in the installed app by

return "export GS_OPTIONS=\\"-sICCProfilesDir=$gs_share/$gs_ver/iccprofiles/ -sGenericResourceDir=$gs_share/$gs_ver/Resource/ -sFontResourceDir=$gs_share/$gs_ver/Resource/Font\\";"

We could export GS_OPTIONS in bundle_octave

@apjanke

This comment has been minimized.

Show comment
Hide comment
@apjanke

apjanke Jun 25, 2018

Contributor

We could export GS_OPTIONS in bundle_octave

Aha, so that's what GS_OPTIONS was for! You had it in there but I removed it because I thought it was a redundancy from constructing the AppleScript. I have restored it, in 9343a80, but the test is still failing.

Here's one of the failures.

>>>>> processing /private/tmp/octave-octave-app_4.4.0-20180625-41714-f1psuf/octave-4.4.0/scripts/plot/util/copyobj.m
***** testif HAVE_MAGICK; any (strcmp ("gnuplot", available_graphics_toolkits ()))
...
png1 = [tempname() ".png"];
   png2 = [tempname() ".png"];
   unwind_protect
     print (h1, png1);
     [img1, map1, alpha1] = imread (png1);
     print (h2, png2);
     [img2, map2, alpha2] = imread (png2);
   unwind_protect_cleanup
     unlink (png1);
     unlink (png2);
   end_unwind_protect
...
!!!!! test failed
imread: unable to find file '/tmp/oct-u0UDYV.png'

Perhaps brew's sandboxing is preventing octave from writing to the global /tmp directory. I'll see if tempname() can be repointed.

Then again, maybe I just didn't get the GS_OPTIONS right; it's still complaining about its initialization file.

  plot/util/copyobj.m .........................................  ./base/gsicc_manage.c:1172: gsicc_open_search(): Could not find default_rgb.icc 
| ./base/gsicc_manage.c:1838: gsicc_set_device_profile(): cannot find device profile
 PASS      1/2   
                                                                  FAIL    1

Hmm. Maybe brew's environment sanitization is deleting the GS_OPTIONS variable, and it needs to be set inside the formula's install block to get it to be effective.

Contributor

apjanke commented Jun 25, 2018

We could export GS_OPTIONS in bundle_octave

Aha, so that's what GS_OPTIONS was for! You had it in there but I removed it because I thought it was a redundancy from constructing the AppleScript. I have restored it, in 9343a80, but the test is still failing.

Here's one of the failures.

>>>>> processing /private/tmp/octave-octave-app_4.4.0-20180625-41714-f1psuf/octave-4.4.0/scripts/plot/util/copyobj.m
***** testif HAVE_MAGICK; any (strcmp ("gnuplot", available_graphics_toolkits ()))
...
png1 = [tempname() ".png"];
   png2 = [tempname() ".png"];
   unwind_protect
     print (h1, png1);
     [img1, map1, alpha1] = imread (png1);
     print (h2, png2);
     [img2, map2, alpha2] = imread (png2);
   unwind_protect_cleanup
     unlink (png1);
     unlink (png2);
   end_unwind_protect
...
!!!!! test failed
imread: unable to find file '/tmp/oct-u0UDYV.png'

Perhaps brew's sandboxing is preventing octave from writing to the global /tmp directory. I'll see if tempname() can be repointed.

Then again, maybe I just didn't get the GS_OPTIONS right; it's still complaining about its initialization file.

  plot/util/copyobj.m .........................................  ./base/gsicc_manage.c:1172: gsicc_open_search(): Could not find default_rgb.icc 
| ./base/gsicc_manage.c:1838: gsicc_set_device_profile(): cannot find device profile
 PASS      1/2   
                                                                  FAIL    1

Hmm. Maybe brew's environment sanitization is deleting the GS_OPTIONS variable, and it needs to be set inside the formula's install block to get it to be effective.

@schoeps

This comment has been minimized.

Show comment
Hide comment
@schoeps

schoeps Jun 25, 2018

Contributor

export TMP=/some/path/that/exists should affect the command tempfile within Octave.

Contributor

schoeps commented Jun 25, 2018

export TMP=/some/path/that/exists should affect the command tempfile within Octave.

@apjanke

This comment has been minimized.

Show comment
Hide comment
@apjanke

apjanke Jun 27, 2018

Contributor

The more I think about this, the more I think this make check is actually in the wrong place, and we shouldn't be adding in environment variables and other settings to make it work. Instead, we should either:

a) be moving the check down after we munge & decorate the app, and test the final app and its wrapper script, or
b) be building a wrapper script as part of the Homebrew octave formula, and test that

Because we should be testing the final octave command as it's presented to the user, and the wrapper script should be taking responsibility for configuring the PATH and GS_OPTIONS and whatnot, and we want to test that mechanism. Setting environment variables as part of the test environment is cheating.

Contributor

apjanke commented Jun 27, 2018

The more I think about this, the more I think this make check is actually in the wrong place, and we shouldn't be adding in environment variables and other settings to make it work. Instead, we should either:

a) be moving the check down after we munge & decorate the app, and test the final app and its wrapper script, or
b) be building a wrapper script as part of the Homebrew octave formula, and test that

Because we should be testing the final octave command as it's presented to the user, and the wrapper script should be taking responsibility for configuring the PATH and GS_OPTIONS and whatnot, and we want to test that mechanism. Setting environment variables as part of the test environment is cheating.

@schoeps

This comment has been minimized.

Show comment
Hide comment
@schoeps

schoeps Jun 27, 2018

Contributor

Agreed!

Contributor

schoeps commented Jun 27, 2018

Agreed!

@schoeps

This comment has been minimized.

Show comment
Hide comment
@schoeps

schoeps Jun 27, 2018

Contributor

The question is how to automize that without a lot of work... the app can open an m-file but it will not execute it. We may need GUI scripting.

Contributor

schoeps commented Jun 27, 2018

The question is how to automize that without a lot of work... the app can open an m-file but it will not execute it. We may need GUI scripting.

@apjanke

This comment has been minimized.

Show comment
Hide comment
@apjanke

apjanke Jun 27, 2018

Contributor

I think the octave --eval CODE command line option would work here. We could use that to point it at a script for execution, and use a little temporary script that just executes the test suite. For example, octave --eval "cd /path/to/our/test/script; octave_app_test_script". Extend our wrapper script to expose the --eval option, and I think this is doable.

Contributor

apjanke commented Jun 27, 2018

I think the octave --eval CODE command line option would work here. We could use that to point it at a script for execution, and use a little temporary script that just executes the test suite. For example, octave --eval "cd /path/to/our/test/script; octave_app_test_script". Extend our wrapper script to expose the --eval option, and I think this is doable.

@schoeps

This comment has been minimized.

Show comment
Hide comment
@schoeps

schoeps Jun 27, 2018

Contributor

We could also add another on run_octave_test function to main.scpt but I am not sure how to call that from shell. I think osascript always executes run.

Contributor

schoeps commented Jun 27, 2018

We could also add another on run_octave_test function to main.scpt but I am not sure how to call that from shell. I think osascript always executes run.

@apjanke

This comment has been minimized.

Show comment
Hide comment
@apjanke

apjanke Jun 27, 2018

Contributor

Perhaps we could introduce a special --run-test-suite argument that main.scpt respects inside run, and have it just do a headless run of the test suite in that case.

Contributor

apjanke commented Jun 27, 2018

Perhaps we could introduce a special --run-test-suite argument that main.scpt respects inside run, and have it just do a headless run of the test suite in that case.

@schoeps

This comment has been minimized.

Show comment
Hide comment
@schoeps

schoeps Jun 27, 2018

Contributor

Ok! we could do something like this

on run_octave_test()
	return "cd ~;clear;/Applications/Octave-4.4.0.app/Contents/Resources/usr/bin/octave --no-window-system --eval '__run_test_suite__' >> ~/octave.txt;exit;"
end run_octave_test

on run argv
	path_check()
	cache_fontconfig()
	if argv contains "--run-test-suite" then
		set cmd to export_gs_options() & export_gnuterm() & export_path() & export_dyld() & run_octave_test()
	else
		set cmd to export_gs_options() & export_gnuterm() & export_path() & export_dyld() & run_octave_gui()
	end if
	do shell script cmd
end run

which allows to run osascript /Applications/Octave-4.4.0.app/Contents/Resources/Scripts/main.scpt '--run-test-suite' which generates a ~/octave.txt.

Contributor

schoeps commented Jun 27, 2018

Ok! we could do something like this

on run_octave_test()
	return "cd ~;clear;/Applications/Octave-4.4.0.app/Contents/Resources/usr/bin/octave --no-window-system --eval '__run_test_suite__' >> ~/octave.txt;exit;"
end run_octave_test

on run argv
	path_check()
	cache_fontconfig()
	if argv contains "--run-test-suite" then
		set cmd to export_gs_options() & export_gnuterm() & export_path() & export_dyld() & run_octave_test()
	else
		set cmd to export_gs_options() & export_gnuterm() & export_path() & export_dyld() & run_octave_gui()
	end if
	do shell script cmd
end run

which allows to run osascript /Applications/Octave-4.4.0.app/Contents/Resources/Scripts/main.scpt '--run-test-suite' which generates a ~/octave.txt.

apjanke added a commit that referenced this issue Jun 27, 2018

Make a new "test" step
This runs the Octave __run_test_suite__ under our wrapper script's environment,
so we can test that setup, too. This replaces the "make check" step in the
octave* formulae.

Addresses #33.
@apjanke

This comment has been minimized.

Show comment
Hide comment
@apjanke

apjanke Jun 27, 2018

Contributor

That sounds like a good idea.

Done in:

Results:

Summary:

  PASS                            14934
  FAIL                                0

Now that's what I'm talkin' about. :)

Contributor

apjanke commented Jun 27, 2018

That sounds like a good idea.

Done in:

Results:

Summary:

  PASS                            14934
  FAIL                                0

Now that's what I'm talkin' about. :)

@apjanke

This comment has been minimized.

Show comment
Hide comment
@apjanke

apjanke Jun 27, 2018

Contributor

Calling this fixed since we have a repeatable clean test run in our bundled app.

Contributor

apjanke commented Jun 27, 2018

Calling this fixed since we have a repeatable clean test run in our bundled app.

@apjanke apjanke closed this Jun 27, 2018

Octave.app automation moved this from High priority to Closed Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment