Add tests for component load order

[tlvu]

This PR should merge to @mishaschwartz PR #304.

Proper component load order is a core feature allowing the user to customize all default behavior of the stack. This is worth to have tests to prevent accidental regressions.

Currently I made the tests passed with the current wrong load order that should be fixed.

Running the tests look like this:

$ ./bash_unit test_read_configs.sh
Running tests in test_read_configs.sh
        Running test_simple_all_conf_dirs ... SUCCESS ✓
        Running test_simple_compose_conf_list ... SUCCESS ✓
Overall result: SUCCESS ✓

The test framework is from https://github.com/pgrange/bash_unit/blob/master/bash_unit.

I also looked at https://github.com/bats-core/bats-core, which seems more popular but also more complex to use and setup. bash_unit is one single file to add while bats-core requires multiple git submodules! KISS for the win.

In order to test the COMPOSE_CONF_LIST, I had to move its logic to read-configs.include.inc. Probably the wrong location and a better location is a brand new .include.inc file. Name suggestion?

I also updated the "Release Instructions" in the README to use make bump major|minor|patch command instead of directly calling bump2version to harmonize with the section "Tagging policy" right above.

[crim-jenkins-bot]

E2E Test Results

DACCS-iac Pipeline Results

Build URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/1386/
Result : failure

BIRDHOUSE_DEPLOY_BRANCH : add-test-for-component-load-order
DACCS_CONFIGS_BRANCH : master
PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master
PAVICS_SDI_BRANCH : master

DESTROY_INFRA_ON_EXIT : true
PAVICS_HOST : https://host-140-67.rdext.crim.ca

PAVICS-e2e-workflow-tests Pipeline Results

Tests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1037/

NOTEBOOK TEST RESULTS

    
[2023-03-26T03:58:18.487Z] ============================= test session starts ==============================
[2023-03-26T03:58:18.487Z] platform linux -- Python 3.8.15, pytest-7.2.0, pluggy-1.0.0
[2023-03-26T03:58:18.487Z] rootdir: /home/jenkins/agent/workspace/PAVICS-e2e-workflow-tests_master
[2023-03-26T03:58:18.487Z] plugins: anyio-3.6.2, dash-2.7.0, nbval-0.9.6, tornasync-0.6.0.post2
[2023-03-26T03:58:18.487Z] collected 268 items
[2023-03-26T03:58:18.487Z] 
[2023-03-26T03:58:25.991Z] notebooks-auth/test_thredds.ipynb ...........                            [  4%]
[2023-03-26T03:58:34.329Z] pavics-sdi-master/docs/source/notebooks/WCS_example.ipynb .......        [  6%]
[2023-03-26T03:58:42.008Z] pavics-sdi-master/docs/source/notebooks/WFS_example.ipynb ......         [  8%]
[2023-03-26T03:58:51.425Z] pavics-sdi-master/docs/source/notebooks/WMS_example.ipynb .F......       [ 11%]
[2023-03-26T03:58:54.159Z] pavics-sdi-master/docs/source/notebooks/WPS_example.ipynb ..........     [ 15%]
[2023-03-26T04:06:36.297Z] pavics-sdi-master/docs/source/notebooks/climex.ipynb ............        [ 20%]
[2023-03-26T04:06:38.202Z] pavics-sdi-master/docs/source/notebooks/eccc-geoapi-climate-stations.ipynb . [ 20%]
[2023-03-26T04:06:43.841Z] ...............                                                          [ 26%]
[2023-03-26T04:06:53.606Z] pavics-sdi-master/docs/source/notebooks/eccc-geoapi-xclim.ipynb .....    [ 27%]
[2023-03-26T04:06:57.686Z] pavics-sdi-master/docs/source/notebooks/esgf-dap.ipynb FFFFFF            [ 30%]
[2023-03-26T04:07:16.756Z] pavics-sdi-master/docs/source/notebooks/forecasts.ipynb ......           [ 32%]
[2023-03-26T04:07:18.150Z] pavics-sdi-master/docs/source/notebooks/jupyter_extensions.ipynb .       [ 32%]
[2023-03-26T04:07:23.752Z] pavics-sdi-master/docs/source/notebooks/opendap.ipynb .......            [ 35%]
[2023-03-26T04:07:29.892Z] pavics-sdi-master/docs/source/notebooks/pavics_thredds.ipynb .....       [ 37%]
[2023-03-26T04:11:21.155Z] pavics-sdi-master/docs/source/notebooks/regridding.ipynb ............... [ 42%]
[2023-03-26T04:12:28.624Z] .............                                                            [ 47%]
[2023-03-26T04:12:33.578Z] pavics-sdi-master/docs/source/notebooks/rendering.ipynb ....             [ 49%]
[2023-03-26T04:12:37.406Z] pavics-sdi-master/docs/source/notebooks/subset-user-input.ipynb ........ [ 52%]
[2023-03-26T04:13:04.696Z] .................                                                        [ 58%]
[2023-03-26T04:13:11.382Z] pavics-sdi-master/docs/source/notebooks/subsetting.ipynb .....           [ 60%]
[2023-03-26T04:13:12.767Z] pavics-sdi-master/docs/source/notebook-components/weaver_example.ipynb . [ 60%]
[2023-03-26T04:13:29.678Z] .........                                                                [ 64%]
[2023-03-26T04:13:40.841Z] finch-master/docs/source/notebooks/dap_subset.ipynb ...........          [ 68%]
[2023-03-26T04:13:50.214Z] finch-master/docs/source/notebooks/finch-usage.ipynb ......              [ 70%]
[2023-03-26T04:14:31.923Z] finch-master/docs/source/notebooks/subset.ipynb ....................ss.  [ 79%]
[2023-03-26T04:14:34.464Z] PAVICS-landing-master/content/notebooks/climate_indicators/PAVICStutorial_ClimateDataAnalysis-1DataAccess.ipynb . [ 79%]
[2023-03-26T04:14:46.370Z] ......                                                                   [ 81%]
[2023-03-26T04:19:22.972Z] PAVICS-landing-master/content/notebooks/climate_indicators/PAVICStutorial_ClimateDataAnalysis-2Subsetting.ipynb . [ 82%]
[2023-03-26T04:20:57.386Z] .............                                                            [ 86%]
[2023-03-26T04:21:15.510Z] PAVICS-landing-master/content/notebooks/climate_indicators/PAVICStutorial_ClimateDataAnalysis-3Climate-Indicators.ipynb . [ 87%]
[2023-03-26T04:26:12.089Z] ....s.                                                                   [ 89%]
[2023-03-26T04:26:20.216Z] PAVICS-landing-master/content/notebooks/climate_indicators/PAVICStutorial_ClimateDataAnalysis-4Ensembles.ipynb . [ 89%]
[2023-03-26T04:26:24.928Z] ...                                                                      [ 91%]
[2023-03-26T04:26:39.832Z] PAVICS-landing-master/content/notebooks/climate_indicators/PAVICStutorial_ClimateDataAnalysis-5Visualization.ipynb . [ 91%]
[2023-03-26T04:27:01.800Z] ......                                                                   [ 93%]
[2023-03-26T04:27:04.238Z] notebooks/hummingbird.ipynb ............                                 [ 98%]
[2023-03-26T04:30:38.465Z] notebooks/stress-tests.ipynb .....                                       [100%]
[2023-03-26T04:30:38.465Z] 
[2023-03-26T04:30:38.465Z] =================================== FAILURES ===================================
    
  

[fmigneault]

I'd suggest defining a requirements file with pytest + pytest-shell-utilities.
Then, it is easier to extend advanced tests and checks with everything pytest supports natively (filtering by markers, collect, report statistics, output, colors, etc.), without worrying about parsing sh specific calls. It also allows us to combine test suites in a language agnostic manner (run python test + bash tests at the same time).

import os
import pytest
from pytestshellutils.shell import Subprocess

CUR_DIR = os.path.abspath(os.path.dirname(__file__))
ROOT_DIR = os.path.dirname(os.path.dirname(CUR_DIR))


@pytest.mark.functional
@pytest.mark.shell
def test_something(shell: Subprocess) -> None:
    env = {"SOME_VAR": ""}
    ret = shell.run("run-some-script.sh", "[param1]", env=env, cwd=ROOT_DIR)
    assert ret.returncode == 0
    assert ret.stderr == ""
    assert "expected output" in ret.stdout
    lines = ret.stdout.splitlines()
    assert len(lines) == 123
    assert lines[-1] == "expected last line"

Consider adding the test suite in some .github/workflows/tests.yml definition to auto-validate it each time.

[fmigneault]

Should be before the proxy and the birds since they depend on these?

[tlvu]

Yes that should hopefully be fixed if my suggested algo change does what I think it does.

[tlvu]

I'd suggest defining a requirements file with pytest + pytest-shell-utilities.
Then, it is easier to extend advanced tests and checks with everything pytest supports natively (filtering by markers, collect, report statistics, output, colors, etc.), without worrying about parsing sh specific calls. It also allows us to combine test suites in a language agnostic manner (run python test + bash tests at the same time).

@fmigneault
Can that pytest-shell-utilities test shell function separately? I basically only unit test the new function source_conf_files and gen_compose_conf_list.

Note we do not want to test the entire pavics-compose.sh script because that would be too heavy (it spins up the entire PAVICS stack). That's what Jenkins is doing. We just want to unit test some core functions to quickly catch regressions. So we just want something lightweight without introducing new requirements (python). Or are you thinking to add python code to the platform code, that is entirely shell script right now?

I see "platform code" as pavics-compose.sh, and read-configs.include.sh for the moment. All the rest are just plugins into multiple hooks that the platform override feature allows.

[fmigneault]

@tlvu
Yes, each test can run its own shell.run("run-some-script.sh" ..., and you can also combine them with pytest.mark.parametrize to run multiple variations of a same test.

I agree we should focus on minimal tests without booting the stack here. We can add more or less items to that test suite as we encounter new problems or add new features.

[tlvu]

Yes, each test can run its own shell.run("run-some-script.sh" ..., and you can also combine them with pytest.mark.parametrize to run multiple variations of a same test.

@fmigneault
Just to be clear are you suggesting replacing bash_unit with pytest or have pytest call bash_unit?

I agree we should focus on minimal tests without booting the stack here. We can add more or less items to that test suite as we encounter new problems or add new features.

bash_unit is very simple and does everything we need at this moment. I suggest we adopt it to quickly moving onto other issues and change it later if needed.

I still have an issue, I had to factor out the gen_compose_conf_list function. I put it in read-configs.include.sh for now but it does not really belongs there. How about a new platform-utils.include.sh?

@mishaschwartz anything you would like to change before merging this into your PR and we will have tests for your PR.

[mishaschwartz]

@tlvu @fmigneault

I agree with @fmigneault that pytest is the better option and should replace bash_unit.

I have taken your the tests from this PR and I'm adapting them into pytests which I will include in #304. I'm mostly done but I won't have time to finish up the tests fully by the end of the day. I'll likely be pushing the changes to the load-components-in-defined-order branch tomorrow morning.

Also... writing the tests have already helped me fix some bugs in the algorithm! Thanks for suggesting these and writing the first drafts!