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

CI: Address Windows specific issues in testsuite #2616

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
939e42a
version dependent import of builtin
ninsbl Oct 28, 2022
0a2f004
test with python3
ninsbl Oct 28, 2022
9ecbb2d
use shell in subprocess on Windows
ninsbl Oct 28, 2022
edea1f9
find grass executable
ninsbl Oct 28, 2022
e2092cb
handle OS newlines in testsuite
ninsbl Oct 28, 2022
c08635c
handle OS newlines in testsuite
ninsbl Oct 28, 2022
fb075bf
handle OS newlines in testsuite
ninsbl Oct 28, 2022
45bb9d7
handle OS newlines in testsuite
ninsbl Oct 28, 2022
6de276a
compare file content not MD5
ninsbl Oct 28, 2022
50ddaad
ignore missing CRS info
ninsbl Oct 28, 2022
590cfd4
remove windows newline from output
ninsbl Oct 28, 2022
a7198dc
black
ninsbl Oct 28, 2022
8951dc3
black
ninsbl Oct 28, 2022
dd1d610
import sys
ninsbl Oct 29, 2022
31a2a3c
limit addons test on Win
ninsbl Oct 29, 2022
1cdc0fd
no proj check on Win
ninsbl Oct 29, 2022
4998aec
import shutil
ninsbl Oct 29, 2022
128d3ea
no proj check on Win
ninsbl Oct 29, 2022
39fc866
drop execution test on win32
ninsbl Oct 31, 2022
7eb95cb
capture shell output on win32
ninsbl Oct 31, 2022
74511ae
use grass tempfile
ninsbl Oct 31, 2022
f89774f
more shell output on windows
ninsbl Oct 31, 2022
1f96842
black
ninsbl Oct 31, 2022
01abed3
gs and file object
ninsbl Nov 1, 2022
0a0d009
type is str
ninsbl Nov 1, 2022
646d238
define _
ninsbl Nov 1, 2022
6b72a10
write bytes
ninsbl Nov 1, 2022
086cc65
import builtins
ninsbl Nov 1, 2022
b219310
remove Python 2
ninsbl Nov 2, 2022
9e7706f
filter is path, flake8
ninsbl Nov 2, 2022
5a3fd3e
windows sep
ninsbl Nov 2, 2022
0114994
do not try to close string
ninsbl Nov 2, 2022
5d4864e
more os sep
ninsbl Nov 2, 2022
80fd808
os.sep in assertEqual for str(Path)
ninsbl Nov 3, 2022
db1473c
black
ninsbl Nov 3, 2022
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
4 changes: 2 additions & 2 deletions gui/wxpython/core/testsuite/toolboxes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
# run make in gui/wxpython before the test
# run test using sh -e

python $GISBASE/gui/wxpython/core/toolboxes.py doctest
python $GISBASE/gui/wxpython/core/toolboxes.py test
python3 $GISBASE/gui/wxpython/core/toolboxes.py doctest
python3 $GISBASE/gui/wxpython/core/toolboxes.py test
Comment on lines +6 to +7
Copy link
Member

Choose a reason for hiding this comment

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

Do we have GRASS_PYTHON defined here or not? If yes, that might be the best choice, although I would prefer solution where plain python just works.

32 changes: 4 additions & 28 deletions gui/wxpython/core/toolboxes.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,6 @@
else:
ETREE_EXCEPTIONS = expat.ExpatError

if sys.version_info[0:2] > (2, 6):
has_xpath = True
else:
has_xpath = False

import grass.script.task as gtask
import grass.script.core as gcore
from grass.script.utils import try_remove, decode
Expand Down Expand Up @@ -430,18 +425,7 @@ def _expandToolboxes(node, toolboxes):
items = n.find("./items")
idx = list(items).index(subtoolbox)

if has_xpath:
toolbox = toolboxes.find(
'.//toolbox[@name="%s"]' % subtoolbox.get("name")
)
else:
toolbox = None
potentialToolboxes = toolboxes.findall(".//toolbox")
sName = subtoolbox.get("name")
for pToolbox in potentialToolboxes:
if pToolbox.get("name") == sName:
toolbox = pToolbox
break
toolbox = toolboxes.find('.//toolbox[@name="%s"]' % subtoolbox.get("name"))

if toolbox is None: # not in file
continue
Expand Down Expand Up @@ -576,15 +560,7 @@ def _expandItems(node, items, itemTag):
"""
for moduleItem in node.findall(".//" + itemTag):
itemName = moduleItem.get("name")
if has_xpath:
moduleNode = items.find('.//%s[@name="%s"]' % (itemTag, itemName))
else:
moduleNode = None
potentialModuleNodes = items.findall(".//%s" % itemTag)
for mNode in potentialModuleNodes:
if mNode.get("name") == itemName:
moduleNode = mNode
break
moduleNode = items.find('.//%s[@name="%s"]' % (itemTag, itemName))

if moduleNode is None: # module not available in dist
continue
Expand Down Expand Up @@ -789,9 +765,9 @@ def new_translator(string):
sys.displayhook = new_displayhook
sys.__displayhook__ = new_displayhook

import __builtin__
import builtins as _builtins

__builtin__._ = new_translator
_builtins.__dict__["_"] = new_translator


def doc_test():
Expand Down
16 changes: 12 additions & 4 deletions lib/gis/testsuite/test_parser_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
@author Soeren Gebbert
"""

import json
import subprocess
import sys

from grass.gunittest.case import TestCase
from grass.script import decode
import json


class TestParserJson(TestCase):
Expand Down Expand Up @@ -50,7 +52,9 @@ def test_r_slope_aspect_json(self):
},
]

stdout, stderr = subprocess.Popen(args, stdout=subprocess.PIPE).communicate()
stdout, stderr = subprocess.Popen(
args, stdout=subprocess.PIPE, shell=sys.platform == "win32"
).communicate()
Comment on lines -53 to +57
Copy link
Member

Choose a reason for hiding this comment

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

This would probably make more sense with direct support for this in grass.script, but to limit the solution only to here, would shutil.which work here at line 22?

print(stdout)
json_code = json.loads(decode(stdout))
self.assertEqual(json_code["module"], "r.slope.aspect")
Expand Down Expand Up @@ -83,7 +87,9 @@ def test_v_out_ascii(self):
}
]

stdout, stderr = subprocess.Popen(args, stdout=subprocess.PIPE).communicate()
stdout, stderr = subprocess.Popen(
args, stdout=subprocess.PIPE, shell=sys.platform == "win32"
).communicate()
print(stdout)
json_code = json.loads(decode(stdout))
self.assertEqual(json_code["module"], "v.out.ascii")
Expand All @@ -99,7 +105,9 @@ def test_v_info(self):
{"param": "layer", "value": "1"},
]

stdout, stderr = subprocess.Popen(args, stdout=subprocess.PIPE).communicate()
stdout, stderr = subprocess.Popen(
args, stdout=subprocess.PIPE, shell=sys.platform == "win32"
).communicate()
print(stdout)
json_code = json.loads(decode(stdout))
self.assertEqual(json_code["module"], "v.info")
Expand Down
18 changes: 12 additions & 6 deletions lib/init/testsuite/test_grass_tmp_mapset.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import os
import shutil
import subprocess

import sys

# Note that unlike rest of GRASS GIS, here we are using unittest package
# directly. The grass.gunittest machinery for mapsets is not needed here.
Expand All @@ -30,13 +30,16 @@ class TestTmpMapset(unittest.TestCase):
"""Tests --tmp-mapset option of grass command"""

# TODO: here we need a name of or path to the main GRASS GIS executable
executable = "grass"
executable = shutil.which("grass")
Copy link
Member

Choose a reason for hiding this comment

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

I would think only shutil.which or shell=True are needed for Windows, not both. I would prefer shutil.which or some equivalent of sys.executable in the grass package (e.g., grass.script.setup.executable).

Copy link
Member Author

Choose a reason for hiding this comment

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

I see. Unfortunately, both shutil.which and shell do not seem to help. Maybe better to switch to subprocess.run?

Copy link
Member

Choose a reason for hiding this comment

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

subprocess.run is just an interface to Popen like all the others. I don't expect it would help. shutil.which needs grass to be on path to work which is likely not fulfilled.

This test is now unique - we don't have other tests which test the executable - we we may have more in the future or even write such code, so perhaps ignoring it now and adding grass.script.setup.executable later is the way to go.

Copy link
Member

Choose a reason for hiding this comment

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

This is one the of things for a separate PR. Getting the grass.script.setup.executable piece working would be nice to have. get_install_path is perhaps a good starting point (Python package path is known, path to installation is not). Lazy-loading seems appropriate, but there are still no properties for modules yet (?), but there is getattr.

# an arbitrary, but identifiable and fairly unique name
location = "test_tmp_mapset_xy"

def setUp(self):
"""Creates a location used in the tests"""
subprocess.check_call([self.executable, "-c", "XY", self.location, "-e"])
subprocess.check_call(
[self.executable, "-c", "XY", self.location, "-e"],
shell=sys.platform == "win32",
)
self.subdirs = os.listdir(self.location)

def tearDown(self):
Expand All @@ -46,7 +49,8 @@ def tearDown(self):
def test_command_runs(self):
"""Check that correct parameters are accepted"""
return_code = subprocess.call(
[self.executable, "--tmp-mapset", self.location, "--exec", "g.proj", "-g"]
[self.executable, "--tmp-mapset", self.location, "--exec", "g.proj", "-g"],
shell=sys.platform == "win32",
)
self.assertEqual(
return_code,
Expand All @@ -67,7 +71,8 @@ def test_command_fails_without_location(self):
"--exec",
"g.proj",
"-g",
]
],
shell=sys.platform == "win32",
)
self.assertNotEqual(
return_code,
Expand All @@ -81,7 +86,8 @@ def test_command_fails_without_location(self):
def test_mapset_metadata_correct(self):
"""Check that metadata is readable and have expected value (XY CRS)"""
output = subprocess.check_output(
[self.executable, "--tmp-mapset", self.location, "--exec", "g.proj", "-g"]
[self.executable, "--tmp-mapset", self.location, "--exec", "g.proj", "-g"],
shell=sys.platform == "win32",
)
self.assertEqual(
output.strip(),
Expand Down
22 changes: 15 additions & 7 deletions python/grass/grassdb/testsuite/test_manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

"""Tests of grass.grassdb.manage"""

import os

from pathlib import Path

from grass.grassdb.manage import MapsetPath, resolve_mapset_path, split_mapset_path
Expand All @@ -33,7 +35,8 @@ def test_mapset_from_path_object(self):
path=full_path, directory=path, location=location_name, mapset=mapset_name
)
# Paths are currently stored as is (not resolved).
self.assertEqual(mapset_path.directory, path)
# pathlib uses os.sep for string represtentation of Path objects
self.assertEqual(mapset_path.directory, path.replace("/", os.sep))
self.assertEqual(mapset_path.location, location_name)
self.assertEqual(mapset_path.mapset, mapset_name)
self.assertEqual(mapset_path.path, Path(path) / location_name / mapset_name)
Expand All @@ -51,7 +54,8 @@ def test_mapset_from_str(self):
mapset=mapset_name,
)
# Paths are currently stored as is (not resolved).
self.assertEqual(mapset_path.directory, path)
# pathlib uses os.sep for string represtentation of Path objects
self.assertEqual(mapset_path.directory, path.replace("/", os.sep))
self.assertEqual(mapset_path.location, location_name)
self.assertEqual(mapset_path.mapset, mapset_name)
self.assertEqual(mapset_path.path, Path(path) / location_name / mapset_name)
Expand All @@ -67,7 +71,7 @@ def test_split_path(self):
ref_mapset = "test_mapset_1"
path = Path(ref_db) / ref_location / ref_mapset
new_db, new_location, new_mapset = split_mapset_path(path)
self.assertEqual(new_db, ref_db)
self.assertEqual(new_db, ref_db.replace("/", os.sep))
self.assertEqual(new_location, ref_location)
self.assertEqual(new_mapset, ref_mapset)

Expand All @@ -78,7 +82,7 @@ def test_split_str(self):
ref_mapset = "test_mapset_1"
path = Path(ref_db) / ref_location / ref_mapset
new_db, new_location, new_mapset = split_mapset_path(str(path))
self.assertEqual(new_db, ref_db)
self.assertEqual(new_db, ref_db.replace("/", os.sep))
self.assertEqual(new_location, ref_location)
self.assertEqual(new_mapset, ref_mapset)

Expand All @@ -89,7 +93,7 @@ def test_split_str_trailing_slash(self):
ref_mapset = "test_mapset_1"
path = Path(ref_db) / ref_location / ref_mapset
new_db, new_location, new_mapset = split_mapset_path(str(path) + "/")
self.assertEqual(new_db, ref_db)
self.assertEqual(new_db, ref_db.replace("/", os.sep))
self.assertEqual(new_location, ref_location)
self.assertEqual(new_mapset, ref_mapset)

Expand Down Expand Up @@ -135,7 +139,9 @@ def test_mapset_from_parts(self):
mapset_path = resolve_mapset_path(
path=path, location=location_name, mapset=mapset_name
)
self.assertEqual(mapset_path.directory, str(Path(path).resolve()))
self.assertEqual(
mapset_path.directory, str(Path(path).resolve()).replace("/", os.sep)
)
self.assertEqual(mapset_path.location, location_name)
self.assertEqual(mapset_path.mapset, mapset_name)
self.assertEqual(
Expand All @@ -149,7 +155,9 @@ def test_mapset_from_path(self):
mapset_name = "test_mapset_1"
full_path = str(Path(path) / location_name / mapset_name)
mapset_path = resolve_mapset_path(path=full_path)
self.assertEqual(mapset_path.directory, str(Path(path).resolve()))
self.assertEqual(
mapset_path.directory, str(Path(path).resolve()).replace("/", os.sep)
)
self.assertEqual(mapset_path.location, location_name)
self.assertEqual(mapset_path.mapset, mapset_name)
self.assertEqual(
Expand Down
7 changes: 7 additions & 0 deletions python/grass/gunittest/case.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,13 @@ def assertLooksLike(self, actual, reference, msg=None):
isinstance(reference, (str, unicode)),
("reference argument is not a string"),
)
if os.linesep != "\n" and os.linesep in reference:
reference = reference.replace(os.linesep, "\n")
if os.linesep != "\n" and os.linesep in actual:
actual = actual.replace(os.linesep, "\n")
# Strip newlines (esp. \r) that may remain after splitting lines
reference = reference.strip()
actual = actual.strip()
if not check_text_ellipsis(actual=actual, reference=reference):
# TODO: add support for multiline (first line general, others with details)
standardMsg = '"%s" does not correspond with "%s"' % (actual, reference)
Expand Down Expand Up @@ -240,6 +245,8 @@ def assertModuleKeyValue(
which is typically obtained using ``-g`` flag.
"""
if isinstance(reference, str):
if os.linesep != "\n" and os.linesep in reference:
reference = reference.replace(os.linesep, "\n")
reference = text_to_keyvalue(reference, sep=sep, skip_empty=True)
module = _module_from_parameters(module, **parameters)
self.runModule(module, expecting_stdout=True)
Expand Down
10 changes: 9 additions & 1 deletion python/grass/gunittest/gmodules.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
:authors: Vaclav Petras, Soeren Gebbert
"""

import os
import subprocess
from grass.script.core import start_command
from grass.script.utils import encode, decode
Expand Down Expand Up @@ -138,4 +139,11 @@ def call_module(
returncode = process.poll()
if returncode:
raise CalledModuleError(module, kwargs, returncode, errors)
return decode(output) if output else None
output = decode(output) if output else None
# Make sure that universal newlines are returned
output = (
output.replace(os.linesep, "\n")
if os.linesep != "\n" and type(output) is str
else output
)
return output
2 changes: 1 addition & 1 deletion python/grass/gunittest/testsuite/test_assertions.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ def setUpClass(cls):
open(cls.emtpy_file, "w").close()
cls.file_with_md5 = cls.__name__ + "_this_is_a_file_with_known_md5"
file_content = "Content of the file with known MD5.\n"
with open(cls.file_with_md5, "w") as f:
with open(cls.file_with_md5, "w", newline="") as f:
f.write(file_content)
# MD5 sum created using:
# echo 'Content of the file with known MD5.' > some_file.txt
Expand Down
2 changes: 1 addition & 1 deletion python/grass/gunittest/testsuite/test_checkers.py
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ def setUpClass(cls):
for line in CORRECT_LINES:
# \n should be converted to platform newline
f.write(line + "\n")
with open(cls.correct_file_name_unix_nl, "w") as f:
with open(cls.correct_file_name_unix_nl, "w", newline="") as f:
for line in CORRECT_LINES:
# binary mode will write pure \n
f.write(line + "\n")
Expand Down
6 changes: 5 additions & 1 deletion python/grass/pygrass/raster/testsuite/test_category.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ def testWrite(self):
cats = Category(self.name)
cats.read()
cats.write_rules(tmpfile)
self.assertFilesEqualMd5(tmpfile, "data/geology_cats")
with open(tmpfile, "r") as act:
actual = act.read()
with open("data/geology_cats", "r") as ref:
reference = ref.read()
self.assertLooksLike(actual, reference)


if __name__ == "__main__":
Expand Down
Loading