Skip to content
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

fixup processing parameters tests #2541

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ci/travis/linux/before_install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@ sudo apt-get install --force-yes --no-install-recommends --no-install-suggests \
python-qt4 \
python-qt4-dev \
python-qt4-sql \
python-qscintilla2 \
python-sip \
python-sip-dev \
python-psycopg2 \
python-numpy \
python-gdal \
spawn-fcgi \
txt2tags \
Expand Down
2 changes: 1 addition & 1 deletion ci/travis/linux/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ cmake --version
${CC} --version

cmake -DWITH_SERVER=ON \
-DWITH_STAGED_PLUGINS=OFF \
-DWITH_STAGED_PLUGINS=ON \
-DWITH_GRASS=ON \
-DSUPPRESS_QT_WARNINGS=ON \
-DENABLE_MODELTEST=ON \
Expand Down
3 changes: 3 additions & 0 deletions ci/travis/osx/before_install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,6 @@ brew ln libffi --force

mkdir -p /Users/travis/Library/Python/2.7/lib/python/site-packages
echo 'import site; site.addsitedir("/usr/local/lib/python2.7/site-packages")' >> /Users/travis/Library/Python/2.7/lib/python/site-packages/homebrew.pth

# Needed for Processing
pip install psycopg2 numpy
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that installed via apt already?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apt on osx? ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops :)

2 changes: 1 addition & 1 deletion ci/travis/osx/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ cd build
#no PGTEST for OSX - can't get postgres to start with brew install
#no APIDOC for OSX - doxygen tests and warnings are covered by linux build
#no deprecated-declarations warnings... requires QGIS ported to Cocoa
cmake -DWITH_SERVER=ON -DWITH_STAGED_PLUGINS=OFF -DWITH_GRASS=OFF \
cmake -DWITH_SERVER=ON -DWITH_STAGED_PLUGINS=ON -DWITH_GRASS=OFF \
-DSUPPRESS_QT_WARNINGS=ON -DENABLE_MODELTEST=ON -DENABLE_PGTEST=OFF \
-DWITH_QWTPOLAR=OFF -DWITH_PYSPATIALITE=ON \
-DQWT_INCLUDE_DIR=/usr/local/opt/qwt/lib/qwt.framework/Headers/ \
Expand Down
70 changes: 27 additions & 43 deletions cmake/UsePythonTest.cmake
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
# Add a python test from a python file # One cannot simply do:
# SET(ENV{PYTHONPATH} ${LIBRARY_OUTPUT_PATH})
# SET(my_test "from test_mymodule import *\;test_mymodule()")
# ADD_TEST(PYTHON-TEST-MYMODULE python -c ${my_test})
# Since cmake is only transmitting the ADD_TEST line to ctest thus you are loosing
# the env var. The only way to store the env var is to physically write in the cmake script
# whatever PYTHONPATH you want and then add the test as 'cmake -P python_test.cmake'
# Add a python test from a python file
#
# Usage:
# SET_SOURCE_FILES_PROPERTIES(test.py PROPERTIES PYTHONPATH
# "${LIBRARY_OUTPUT_PATH}:${VTK_DIR}")
# ADD_PYTHON_TEST(PYTHON-TEST test.py)
#
# Optionally pass environment variables to your test
# ADD_PYTHON_TEST(PYTHON-TEST test.py FOO="bar" BAZ="quux")
#
# Copyright (c) 2006-2010 Mathieu Malaterre <mathieu.malaterre@gmail.com>
#
# Redistribution and use is allowed according to the terms of the New
Expand All @@ -24,45 +19,34 @@ MARK_AS_ADVANCED(PYTHON_EXECUTABLE)

MACRO(ADD_PYTHON_TEST TESTNAME FILENAME)
GET_SOURCE_FILE_PROPERTY(loc ${FILENAME} LOCATION)
GET_SOURCE_FILE_PROPERTY(pyenv ${FILENAME} PYTHONPATH)
SET(EXTRA_ENVIRONMENT ${ARGN})

IF(CMAKE_CONFIGURATION_TYPES)
ADD_TEST(NAME ${TESTNAME} COMMAND ${PYTHON_EXECUTABLE} ${loc} -D CMAKE_BUILD_TYPE=$<CONFIGURATION>)
ELSE(CMAKE_CONFIGURATION_TYPES)
ADD_TEST(NAME ${TESTNAME} COMMAND ${PYTHON_EXECUTABLE} ${loc})
ENDIF(CMAKE_CONFIGURATION_TYPES)

SET(UNIX_TEST_PYTHONPATH "${QGIS_OUTPUT_DIRECTORY}/python:${QGIS_OUTPUT_DIRECTORY}/python/plugins:${CMAKE_SOURCE_DIR}/tests/src/python:$ENV{PYTHONPATH}")
IF(WIN32)
STRING(REGEX REPLACE ":" " " wo_semicolon "${ARGN}")
# Separate Windows PATHs with semicolons
STRING(REGEX REPLACE ":" "\\\;" TEST_PYTHONPATH ${UNIX_TEST_PYTHONPATH})
SET(TEST_QGIS_PREFIX_PATH "${QGIS_OUTPUT_DIRECTORY}/bin/${CMAKE_BUILD_TYPE}")
SET(TEST_PATH "${QGIS_OUTPUT_DIRECTORY}/bin/${CMAKE_BUILD_TYPE};$ENV{PATH}")
SET(ENVIRONMENT_VARS "PYTHONPATH=${TEST_PYTHONPATH}" "QGIS_PREFIX_PATH=${TEST_QGIS_PREFIX_PATH}" "PATH=${TEST_PATH}")
ELSE(WIN32)
STRING(REGEX REPLACE ";" " " wo_semicolon "${ARGN}")
SET(TEST_PYTHONPATH ${UNIX_TEST_PYTHONPATH})
SET(TEST_QGIS_PREFIX_PATH "${QGIS_OUTPUT_DIRECTORY}")
SET(TEST_LD_LIBRARY_PATH "${QGIS_OUTPUT_DIRECTORY}/lib:$ENV{LD_LIBRARY_PATH}")
SET(ENVIRONMENT_VARS "PYTHONPATH=${TEST_PYTHONPATH}" "QGIS_PREFIX_PATH=${TEST_QGIS_PREFIX_PATH}" "LD_LIBRARY_PATH=${TEST_LD_LIBRARY_PATH}")
ENDIF(WIN32)
SET(TEST_ENVIRONMENT_VARS ${ENVIRONMENT_VARS} ${EXTRA_ENVIRONMENT})

FILE(WRITE ${CMAKE_CURRENT_BINARY_DIR}/${TESTNAME}.cmake
"
IF(WIN32)
SET(ENV{QGIS_PREFIX_PATH} \"${QGIS_OUTPUT_DIRECTORY}/bin/\${CMAKE_BUILD_TYPE}\")
SET(ENV{PATH} \"${QGIS_OUTPUT_DIRECTORY}/bin/\${CMAKE_BUILD_TYPE};\$ENV{PATH}\")
SET(ENV{PYTHONPATH} \"${QGIS_OUTPUT_DIRECTORY}/python/;\$ENV{PYTHONPATH}\")
MESSAGE(\"PATH:\$ENV{PATH}\")
ELSE(WIN32)
SET(ENV{QGIS_PREFIX_PATH} \"${QGIS_OUTPUT_DIRECTORY}\")
SET(ENV{LD_LIBRARY_PATH} \"${pyenv}:${QGIS_OUTPUT_DIRECTORY}/lib:\$ENV{LD_LIBRARY_PATH}\")
SET(ENV{PYTHONPATH} \"${QGIS_OUTPUT_DIRECTORY}/python/:\$ENV{PYTHONPATH}\")
MESSAGE(\"LD_LIBRARY_PATH:\$ENV{LD_LIBRARY_PATH}\")
ENDIF(WIN32)

MESSAGE(\"PYTHONPATH:\$ENV{PYTHONPATH}\")
MESSAGE(STATUS \"Running ${PYTHON_EXECUTABLE} ${loc} ${wo_semicolon}\")
EXECUTE_PROCESS(
COMMAND ${PYTHON_EXECUTABLE} ${loc} ${wo_semicolon}
RESULT_VARIABLE import_res
)
# Pass the output back to ctest
IF(import_res)
MESSAGE(FATAL_ERROR \"Test failed: \${import_res}\")
ENDIF(import_res)
"
)
IF(CMAKE_CONFIGURATION_TYPES)
ADD_TEST(NAME ${TESTNAME} COMMAND ${CMAKE_COMMAND} -D CMAKE_BUILD_TYPE=$<CONFIGURATION> -P ${CMAKE_CURRENT_BINARY_DIR}/${TESTNAME}.cmake)
ELSE(CMAKE_CONFIGURATION_TYPES)
ADD_TEST(NAME ${TESTNAME} COMMAND ${CMAKE_COMMAND} -P ${CMAKE_CURRENT_BINARY_DIR}/${TESTNAME}.cmake)
ENDIF(CMAKE_CONFIGURATION_TYPES)
SET_PROPERTY(TEST ${TESTNAME} PROPERTY ENVIRONMENT ${TEST_ENVIRONMENT_VARS})
# For Debug
#FOREACH(arg ${TEST_ENVIRONMENT_VARS})
# MESSAGE("Test ENV: ${arg}")
#ENDFOREACH()
ENDMACRO(ADD_PYTHON_TEST)

# Byte compile recursively a directory (DIRNAME)
Expand Down
4 changes: 3 additions & 1 deletion python/plugins/processing/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ ADD_SUBDIRECTORY(images)
ADD_SUBDIRECTORY(modeler)
ADD_SUBDIRECTORY(script)
ADD_SUBDIRECTORY(tools)
ADD_SUBDIRECTORY(tests)
ADD_SUBDIRECTORY(ui)
IF (ENABLE_TESTS)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you propose to keep plugin tests in their directories?
It's fine for me, just wondering about the reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right - my reasoning is kind of abstract, so bear with me...

Firstly, from a software patterns POV, "plugins" should be self contained. One small example of this is that moving plugins in and out of core is simpler if the plugin carries and installs it's own tests.

And more abstractly, I've been thinking of plugins as python modules. Even though they aren't all currently built this way, I think we should move towards that. We're using python and the standard unit of application composition in Python is a module. The tests are part of the module and "/tests or /tests.py" is a standard place to store a modules tests.

Finally, I think core plugins should serve as an example for contributed plugins, and they should resemble contributed plugins as much as possible so that in core plugins we're dogfooding the same plugin tooling and practices that we want contributors to use. To my point - part of the plugin code is (or should be) tests, and I hope we can move more contributors to write tests for their plugin by making it easy to run plugin tests in situ. This plugin doesn't solve that, but I think moving Processing's tests out of its module would be a step in the wrong direction.

ADD_SUBDIRECTORY(tests)
ENDIF (ENABLE_TESTS)

FILE(GLOB OTHER_FILES metadata.txt)
FILE(GLOB PY_FILES *.py)
Expand Down
19 changes: 11 additions & 8 deletions python/plugins/processing/core/parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,14 +220,17 @@ def getValueAsCommandLineParameter(self):
return '"' + unicode(self.value) + '"'

def setValue(self, obj):
self.value = unicode(obj)
if self.value.strip() == '' or self.value is None:
if obj == None or unicode(obj).strip() == '':
if not self.optional:
return False
self.value = ''
if self.ext is not None and self.value != '':
return self.value.endswith(self.ext)
return True
else:
self.value = ''
return True
else:
self.value = unicode(obj)
if self.ext is not None:
return self.value.endswith(self.ext)
return True

def typeName(self):
if self.isFolder:
Expand Down Expand Up @@ -384,9 +387,9 @@ def getAsString(self, value):
if layer.name() == s:
return unicode(layer.dataProvider().dataSourceUri())
return s
if self.datatype == ParameterMultipleInput.TYPE_FILE:
elif self.datatype == ParameterMultipleInput.TYPE_FILE:
return unicode(value)
else:
else: # Some vector type
if isinstance(value, QgsVectorLayer):
return unicode(value.source())
else:
Expand Down
2 changes: 1 addition & 1 deletion python/plugins/processing/gui/ProcessingToolbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def __init__(self):

self.modeComboBox.clear()
self.modeComboBox.addItems([self.tr('Simplified interface'),
self.tr('Advanced interface')])
self.tr('Advanced interface')])
settings = QSettings()
if not settings.contains(self.USE_CATEGORIES):
settings.setValue(self.USE_CATEGORIES, True)
Expand Down
4 changes: 4 additions & 0 deletions python/plugins/processing/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,7 @@ FILE(GLOB TEST_DATA_FILES data/*)

PLUGIN_INSTALL(processing tests ${PY_FILES})
PLUGIN_INSTALL(processing tests/data ${TEST_DATA_FILES})

INCLUDE(UsePythonTest)

ADD_PYTHON_TEST(ProcessingParametersTest ParametersTest.py)
Loading