support/run-test: move pacakges' tests to packages' directories

Move the existing few packages' tests to the correspnding package

directories.

If we were to add the package/ directory to nose2's search path, we would have to add __init__.py files about everywhere, at each level of the directory hierarchy, because nose2 only includes tests that are in a python pacakge (a directory with a __init__.py file is a python package).

This is far from ideal, especially since we have absolutely nothing to put in those __init__.py files.

An alternative is to just scan the package/ directory ourselves, and create symlinks in a sub-directory of support/tessting/tests/ so that

nose2 does find them. We have no two packages with the same name, we don't risk having two symlinks with the same name.

That second solution is what we choose to do, at the cost of a slight increase in complexity in the run-test script.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Ricardo Martincoski <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

Cc Samuel Martin <[email protected]>

---
 .gitignore                                         |  1 +
 .../dropbear/dropbear.py                           |  0
 .../python-ipython/python-ipython.py               |  0
 .../test_python.py => package/python/python.py     |  0
 support/testing/run-tests                          | 22 ++++++++++++++++++++++
 support/testing/tests/package/__init__.py          |  0
 6 files changed, 23 insertions(+)
 rename support/testing/tests/package/test_dropbear.py => package/dropbear/dropbear.py (100%)
 rename support/testing/tests/package/test_ipython.py => package/python-ipython/python-ipython.py (100%)

Quoted 10 lines

---
 .gitignore                                         |  1 +
 .../dropbear/dropbear.py                           |  0
 .../python-ipython/python-ipython.py               |  0
 .../test_python.py => package/python/python.py     |  0
 support/testing/run-tests                          | 22 ++++++++++++++++++++++
 support/testing/tests/package/__init__.py          |  0
 6 files changed, 23 insertions(+)
 rename support/testing/tests/package/test_dropbear.py => package/dropbear/dropbear.py (100%)
 rename support/testing/tests/package/test_ipython.py => package/python-ipython/python-ipython.py (100%)

As I mentioned in another thread, having Python files with '-' in their name AFAIK is not a problem as long we don't want to import them.
We currently import only test_python.py and it seems unlikely to have runtime tests importing each other.

And this series is really changing the filename convention for runtime tests of packages, making the pre-existing convention of package files beat the convention inside the test infra. I am OK with this change, but I think it worth mentioning. Perhaps it fits in the commit log?

[snip]

support/testing/run-tests def main(): · line 42

Quoted 5 lines

support/testing/run-tests +1 -0

@@ -36,6 +39,9 @@ def main():
     script_path = os.path.realpath(__file__)
     test_dir = os.path.dirname(script_path)
 
+    gatheradditionaltests(os.path.abspath("."),

Below you run all files in this path (base dir of buildroot) to match against the regex. It's bad because one can have output/, test-output/, test-dl/, outgoing/, ...

Also os.getcwd() should give the same result and it is already used in support/testing/infra/__init__.py

So for now I suggest to change to:
gather_additional_tests(os.path.join(os.getcwd(), "package"),

And if fs tests gets moved, just add another call to the same method. We don't have that many subdirectories in the root directory.

Other way around is to pass an array of relative subdirectories gather_additional_tests(["package", "fs"],
and compose the absolute path inside the method while iterating over the entries.

support/testing/run-tests def main(): · line 43

Quoted 1 line

support/testing/run-tests +1 -0

+                          os.path.join(test_dir,"tests","package"))

Here and all over the file there are missing whitespace after ',' Can you use flake8 to find them and fix it?

If you use vim you can do
:set makeprg=flake8\ %
:make
and it will jump the cursor to the right place

Quoted 4 lines

support/testing/run-tests +1 -0

+
     if args.stdout:
         BRTest.logtofile = False
 

support/testing/run-tests def main(): · line 125

Quoted 5 lines

support/testing/run-tests +1 -0

@@ -116,5 +122,21 @@ def main():
 
     nose2.discover(argv=nose2_args)
 
+def gatheradditionaltests(search_dir, symlink_dir):

You could call it gather_additional_tests.

support/testing/run-tests def main(): · line 131

Quoted 6 lines

support/testing/run-tests +6 -0

+    try:
+        shutil.rmtree(symlink_dir)
+    except OSError as err:
+        if not err.errno == errno.ENOENT:
+            raise err
+        pass

Could we do just this? (like is done in builder.py)
if os.path.exists(symlink_dir):
shutil.rmtree(symlink_dir)
If you change this, errno doesn't need to be imported.

support/testing/run-tests def main(): · line 134

Quoted 3 lines

support/testing/run-tests +3 -0

+    os.mkdir(symlink_dir)
+    open(os.path.join(symlink_dir,"__init__.py"), "w").close()
+    for dir, _, files in os.walk(os.path.abspath(search_dir)):

I first thought, why not glob.glob? But in Python 2 it is not recursive. So indeed os.walk is the way to go.

support/testing/run-tests def main(): · line 136

Quoted 2 lines

support/testing/run-tests +2 -0

+        package = os.path.basename(dir)
+        for file in files:

You can import fnmatch to drastically reduce the number of regex compilations:
for file in fnmatch.filter(files, '*.py'):

support/testing/run-tests def main(): · line 139

Quoted 3 lines

support/testing/run-tests +3 -0

+            if re.match("^{}.py$".format(package),file):
+                os.symlink(os.path.join(dir,file),
+                           os.path.join(symlink_dir,"test_{}.py".format(package)))

Regards,
Ricardo

On 2017-09-17 02:20 -0300, Ricardo Martincoski spake thusly:

Quoted 1 line · Yann E. MORIN

Quoted 5 lines

support/testing/run-tests def main(): · line 136

Quoted 2 lines

support/testing/run-tests +2 -0

+        package = os.path.basename(dir)
+        for file in files:

You can import fnmatch to drastically reduce the number of regex compilations:
for file in fnmatch.filter(files, '*.py'):

Wee! :-)

On 2017-09-17 09:18 +0200, Yann E. MORIN spake thusly:

Quoted 2 lines

On 2017-09-17 02:20 -0300, Ricardo Martincoski spake thusly:

Quoted 1 line · Yann E. MORIN

[--SNIP--]

Quoted 3 lines

Quoted 3 lines

Quoted 2 lines

support/testing/run-tests +2 -0

+        package = os.path.basename(dir)
+        for file in files:

Quoted 3 lines

Quoted 2 lines

You can import fnmatch to drastically reduce the number of regex compilations:
for file in fnmatch.filter(files, '*.py'):

Wee! :-)

In fact, no. At least, not in this state, because we do not want to match _any_ .py file; instead, we want to match only .py files that are named after the package.

But I guess I can make it to work with fnmatch nonetheless. ;-)

Quoted 7 lines

On 2017-09-17 09:18 +0200, Yann E. MORIN spake thusly:

Quoted 2 lines

On 2017-09-17 02:20 -0300, Ricardo Martincoski spake thusly:

Quoted 1 line · Yann E. MORIN

[--SNIP--]

Quoted 3 lines

Quoted 3 lines

Quoted 2 lines

support/testing/run-tests +2 -0

+        package = os.path.basename(dir)
+        for file in files:

Quoted 7 lines

Quoted 3 lines

Quoted 2 lines

You can import fnmatch to drastically reduce the number of regex compilations:
for file in fnmatch.filter(files, '*.py'):

Wee! :-)

In fact, no. At least, not in this state, because we do not want to match _any_ .py file; instead, we want to match only .py files that are named after the package.

We could use 2 filters. Instead of 8k+ regex compilations (one for each file inside a package directory) we would run 2k+ fnmatch.filter (that probably compile one regex, one per directory) and one regex compilation for each test file in the package tree.

for file in fnmatch.filter(files, '*.py'):
if re.match("^{}.py$".format(package),file):

But well... the default python interpreter has a cache for regex (I don't know its internals, just that it exists) and perhaps the change I suggested won't make any difference in performance.

Quoted 2 lines

But I guess I can make it to work with fnmatch nonetheless. ;-)

BTW, could you change the DEVELOPERS file in the same patch?
+F: package/*/*.py
F: support/testing/

On 2017-10-01 21:54 -0300, Ricardo Martincoski spake thusly:

Quoted 21 lines · Yann E. MORIN

Quoted 7 lines

On 2017-09-17 09:18 +0200, Yann E. MORIN spake thusly:

Quoted 2 lines

On 2017-09-17 02:20 -0300, Ricardo Martincoski spake thusly:

Quoted 1 line · Yann E. MORIN

[--SNIP--]

Quoted 3 lines

Quoted 3 lines

Quoted 2 lines

support/testing/run-tests +2 -0

+        package = os.path.basename(dir)
+        for file in files:

Quoted 7 lines

Quoted 3 lines

Quoted 2 lines

You can import fnmatch to drastically reduce the number of regex compilations:
for file in fnmatch.filter(files, '*.py'):

Wee! :-)

In fact, no. At least, not in this state, because we do not want to match _any_ .py file; instead, we want to match only .py files that are named after the package.

We could use 2 filters. Instead of 8k+ regex compilations (one for each file inside a package directory) we would run 2k+ fnmatch.filter (that probably compile one regex, one per directory) and one regex compilation for each test file in the package tree.

Quoted 2 lines

for file in fnmatch.filter(files, '*.py'):
if re.match("^{}.py$".format(package),file):

Err... I already changed it to:

for file in fnmatch.filter(files, '{}.py'.format(package)):

Is there something wrong with that? ;-)

Quoted 6 lines

But well... the default python interpreter has a cache for regex (I don't know its internals, just that it exists) and perhaps the change I suggested won't make any difference in performance.

Quoted 1 line

Quoted 5 lines

Quoted 1 line

But I guess I can make it to work with fnmatch nonetheless. ;-)

BTW, could you change the DEVELOPERS file in the same patch?
+F: package/*/*.py
F: support/testing/

You meant, add it to your entry ?

In fact, I would expect to assign the test files to the corresponding package custodian (now imp[licit by their location).

But yes, I can assign them to you as well.

testing: add python-cryptography tests

From: Yegor Yefremov <[email protected]>

Signed-off-by Yegor Yefremov <[email protected]>

Reviewed-by Ricardo Martincoski <[email protected]>

[yann.morin.1998 at free.fr:
- move to package directory,

- indent the config fragment
]

Signed-off-by Yann E. MORIN <[email protected]>

---
Note:
This is exactly the same as https://patchwork.ozlabs.org/patch/811011/
but with the test moved to the python-cryptography package and the config fragment indented.

---
 .gitlab-ci.yml                                     |  2 ++
 package/python-cryptography/python-cryptography.py | 31 ++++++++++++++++++++++
 2 files changed, 33 insertions(+)
 create mode 100644 package/python-cryptography/python-cryptography.py

.gitlab-ci.yml +0 -0

index b44c935f2a..091b016191 100644

Quoted 2 lines

testing: add python-cryptography tests

This patch could now be renamed to:
python-cryptography: add runtime tests
or something like this

Quoted 6 lines

From: Yegor Yefremov <[email protected]>

Signed-off-by Yegor Yefremov <[email protected]>

Reviewed-by Ricardo Martincoski <[email protected]>

[yann.morin.1998 at free.fr:
- move to package directory,

- rename the test file to follow the package convention,

I am OK with this change, but I think it worth mentioning.

Quoted 9 lines

- indent the config fragment
]

Signed-off-by Yann E. MORIN <[email protected]>

---
Note:
This is exactly the same as https://patchwork.ozlabs.org/patch/811011/
but with the test moved to the python-cryptography package and the config fragment indented.

Quoted 5 lines

---
 .gitlab-ci.yml                                     |  2 ++
 package/python-cryptography/python-cryptography.py | 31 ++++++++++++++++++++++
 2 files changed, 33 insertions(+)
 create mode 100644 package/python-cryptography/python-cryptography.py

.gitlab-ci.yml needs to be generated again because of this rename (to use '-' in the name again).

Quoted 6 lines

.gitlab-ci.yml +0 -0

index b44c935f2a..091b016191 100644

.gitlab-ci.yml tests.package.test_ipython.TestIPythonPy2: *runtime_test · line 257

Quoted 8 lines

.gitlab-ci.yml +2 -0

@@ -250,6 +250,8 @@ tests.package.test_ipython.TestIPythonPy2: *runtime_test
 tests.package.test_ipython.TestIPythonPy3: *runtime_test
 tests.package.test_python.TestPython2: *runtime_test
 tests.package.test_python.TestPython3: *runtime_test
+tests.package.test_python_cryptography.TestPythonPy2Cryptography: *runtime_test
+tests.package.test_python_cryptography.TestPythonPy3Cryptography: *runtime_test
 tests.toolchain.test_external.TestExternalToolchainBuildrootMusl: *runtime_test
 tests.toolchain.test_external.TestExternalToolchainBuildrootuClibc: *runtime_test
 tests.toolchain.test_external.TestExternalToolchainCCache: *runtime_test

[snip]

Regards,
Ricardo