Reorganising test directories #287

mlange05 · 2024-04-12T13:47:39Z

The next step in the repository spring clean (issue #253 ) is to move the test battery inside the actual loki package and reorganise them to move the respective test tests into the appropriate sub-packages (eg. loki.batch.tests or loki.transform.tests.

This PR does this for all obvious cases, to get the bulk out of the way, leaving the more subtle cases for later and a potentially more detailed refactoring. Importantly, I have resolved the tests/conftest.py and moved all the utilities into the respective sub-packages, to ensure imports for multi-sub-directory testing works.
As a result, we can now do cute things like this:


(loki_env) $ py.test --pyargs loki.batch
======================================================================================================== test session starts ========================================================================================================
platform linux -- Python 3.8.8, pytest-7.3.1, pluggy-1.0.0
rootdir: /lus/h2resw01/hpcperm/naml/loki
configfile: pyproject.toml
plugins: anyio-3.6.2, cov-4.0.0
collected 162 items                                                                                                                                                                                                                 

loki/batch/tests/test_batch.py ..................................................................                                                                                                                             [ 40%]
loki/batch/tests/test_scheduler.py ............................................x.x.................................................                                                                                           [100%]

================================================================================================== 160 passed, 2 xfailed in 51.56s ==================================================================================================

This PR is very invasive, but most of the changes are mechanical file moves and the necessary import changes. I used this opportunity to do a general import clean-up across the test battery, trying to enforce a few common conventions and doing imports mainly from the respective sub-packages (eg. from loki.expression import symbols as sym and from loki.ir import nodes as ir. This causes the second source of a large amount of generic line changes.

The third set of source changes pertains to the location of the (sometimes shared) raw source files in the test directory. Where necessary, I introduced a fixture testdir that points to the root test package loki.tests. Eventually we can selectively move some of these into the respective sub-package test directories, but that is beyond this PR.

Importantly, I've also not touched the transformations sub-package, as the consolidation with loki.transform will be more subtle in many way. Restructuring and re-integrating that is also left to a follow-on PR.

In a little more detail:

Move pragma_utils into loki.ir
Moved all tests into loki.tests
Removed conftests.py and moved available_frontends into loki.frontend, the JIT and test-cleaning utilities into loki.build.jit and the remainder into loki.tools.util
Moved respective tests into subpackages for loki.ir, loki.expression, loki.batch, loki.frontend, loki.backend, loki.build, loki.analysis, loki.tools and, of course, loki.transform
Introduced testdir fixture to point to the root testdir for external source file paths where needed
Refactored import lists across tests touched (apologies, I might have I missed a few) to use a roughly common import convention

github-actions · 2024-04-12T13:49:52Z

Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/287/index.html

codecov · 2024-04-12T15:12:39Z

Codecov Report

Attention: Patch coverage is 95.59939% with 29 lines in your changes are missing coverage. Please review.

Project coverage is 94.93%. Comparing base (b5c5791) to head (07c2f40).

Files	Patch %	Lines
loki/tools/util.py	32.50%	27 Missing ⚠️
loki/build/jit.py	96.61%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #287      +/-   ##
==========================================
+ Coverage   92.89%   94.93%   +2.03%     
==========================================
  Files         102      152      +50     
  Lines       18334    31411   +13077     
==========================================
+ Hits        17032    29820   +12788     
- Misses       1302     1591     +289

Flag	Coverage Δ
lint_rules	`96.39% <ø> (ø)`
loki	`95.10% <95.59%> (+2.22%)`	⬆️
transformations	`92.11% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

reuterbal

Thanks, this is excellent!

Given the guarantee for this to introduce conflicts I would like to bring this in asap. I left a remark about one of the utilities coming from conftest, otherwise nothing that needs changing from my side.

reuterbal · 2024-04-15T11:15:26Z

loki/build/jit.py

+    pymod = compile_and_load(testname, cwd=str(refpath.parent), use_f90wrap=True, f90wrap_kind_map=_f90wrap_kind_map)
+
+    if modulename:
+        # modname = '_'.join(s.capitalize() for s in refpath.stem.split('_'))


debug leftover?

reuterbal · 2024-04-15T11:16:14Z

loki/build/jit.py

+    return lib.wrap(modname=name, sources=wrap, builder=builder, kind_map=_f90wrap_kind_map)
+
+
+def generate_identity(refpath, routinename, modulename=None, frontend=OFP):


Do we actually use this utility somewhere? The default value for frontend is OFP (sic!) and codecov doesn't pick up on it being used.

Yes, this does indeed seem obsolete these days. Will remove - good spot!

reuterbal · 2024-04-15T11:21:27Z

loki/tests/test_cmake.py

@@ -197,7 +195,7 @@ def test_cmake_plan(srcdir, config, cmake_project, loki_install, ecbuild, silent
    assert cmake_project.exists()

    for loki_root in loki_install:
-        with clean_builddir('test_cmake_plan') as builddir:
+        with clean_builddir('test_cmake_plan') as builddir:            


I'm surprised pylint didn't pick up on this, but looks like empty white space at the end of the line?

mlange05 · 2024-04-15T13:30:03Z

@reuterbal Ok, I pushed the changes as a single top-level commit and then rebased to remove the conflicts. 🤞

reuterbal

Thanks, looks good to me now!

mlange05 requested a review from reuterbal April 12, 2024 13:47

mlange05 force-pushed the naml-repo-reorg-part2 branch 2 times, most recently from a9a405c to 0e1de19 Compare April 12, 2024 14:43

mlange05 marked this pull request as ready for review April 12, 2024 15:15

reuterbal requested changes Apr 15, 2024

View reviewed changes

mlange05 added 15 commits April 15, 2024 13:15

Loki: Move pragma_utils into loki.ir

794f386

Loki: Move tests to inside the main package

8b3f382

Frontend: Moving available_frontends utility to loki.frontend

c7c0bd3

Build: Moving JIT and test cleaning utilities to loki.build.jit

a54d7e0

Tools: Remove conftest and move remaining utilities to loki.tools.util

0624bc0

Backend: Moving tests into sub-package test-directory

bea9eda

Frontend: Move test into sub-package directory and massage imports

8a85d70

Expression: Move tests into sub-package directory and massage imports

7a72159

IR: Move tests into sub-package directory and massaging imports

a62bc31

Analyse+Tools: Move tests into sub-package directory

18486ea

Transform: Move tests into sub-package directory and massage imports

5b2da21

Batch: Move tests into sub-package directory and massage imports

3158dde

Build: Move test directory into sub-package directory

0aa2dfa

Lint: Move test directory into sub-pacakge directory

b5f3343

Tools: Removing unused generate_identity utility

739c8c4

mlange05 force-pushed the naml-repo-reorg-part2 branch from 0e1de19 to 739c8c4 Compare April 15, 2024 13:29

Loki: Appease the linting gods...

07c2f40

reuterbal approved these changes Apr 16, 2024

View reviewed changes

reuterbal added the ready for merge This PR has been approved and is ready to be merged label Apr 16, 2024

reuterbal merged commit 4068b82 into main Apr 16, 2024
12 checks passed

reuterbal deleted the naml-repo-reorg-part2 branch April 16, 2024 06:53

reuterbal mentioned this pull request Apr 16, 2024

[Bugfix] available_frontends: Import pytest locally to make dependency optional #290

Merged

mlange05 restored the naml-repo-reorg-part2 branch April 23, 2024 12:23

mlange05 deleted the naml-repo-reorg-part2 branch April 23, 2024 12:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorganising test directories #287

Reorganising test directories #287

mlange05 commented Apr 12, 2024 •

edited

Loading

github-actions bot commented Apr 12, 2024

codecov bot commented Apr 12, 2024 •

edited

Loading

reuterbal left a comment

reuterbal Apr 15, 2024

reuterbal Apr 15, 2024

mlange05 Apr 15, 2024

reuterbal Apr 15, 2024

mlange05 commented Apr 15, 2024

reuterbal left a comment

		return lib.wrap(modname=name, sources=wrap, builder=builder, kind_map=_f90wrap_kind_map)


		def generate_identity(refpath, routinename, modulename=None, frontend=OFP):

Reorganising test directories #287

Reorganising test directories #287

Conversation

mlange05 commented Apr 12, 2024 • edited Loading

github-actions bot commented Apr 12, 2024

codecov bot commented Apr 12, 2024 • edited Loading

Codecov Report

reuterbal left a comment

Choose a reason for hiding this comment

reuterbal Apr 15, 2024

Choose a reason for hiding this comment

reuterbal Apr 15, 2024

Choose a reason for hiding this comment

mlange05 Apr 15, 2024

Choose a reason for hiding this comment

reuterbal Apr 15, 2024

Choose a reason for hiding this comment

mlange05 commented Apr 15, 2024

reuterbal left a comment

Choose a reason for hiding this comment

mlange05 commented Apr 12, 2024 •

edited

Loading

codecov bot commented Apr 12, 2024 •

edited

Loading