From d86debf2ee3d70ffbbccddd392266b43f81dae24 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 17 Jan 2024 10:54:30 +0000 Subject: [PATCH 1/4] Fix test and no need to be root Signed-off-by: Pedro Algarvio --- tests/pytests/integration/states/test_file.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/pytests/integration/states/test_file.py b/tests/pytests/integration/states/test_file.py index ec8aafb299d..afc3d721c0c 100644 --- a/tests/pytests/integration/states/test_file.py +++ b/tests/pytests/integration/states/test_file.py @@ -384,15 +384,12 @@ def test_issue_50221( assert target_path.read_text().replace("\r\n", "\n") == expected_content -@pytest.mark.skip_if_not_root -@pytest.mark.usefixtures("pillar_tree") def test_issue_60426( salt_master, salt_call_cli, tmp_path, - salt_minion, ): - target_path = tmp_path / "/etc/foo/bar" + target_path = tmp_path.joinpath("etc/foo/bar") jinja_name = "foo_bar" jinja_contents = ( "{% for item in accumulator['accumulated configstuff'] %}{{ item }}{% endfor %}" @@ -453,6 +450,7 @@ def test_issue_60426( - filename: {target_path} - text: - some + - more - good - stuff - watch_in: @@ -464,7 +462,6 @@ def test_issue_60426( sls_tempfile = salt_master.state_tree.base.temp_file( "{}.sls".format(sls_name), sls_contents ) - jinja_tempfile = salt_master.state_tree.base.temp_file( "{}.jinja".format(jinja_name), jinja_contents ) @@ -478,7 +475,7 @@ def test_issue_60426( # Check to make sure the file was created assert target_path.is_file() # The type of new line, ie, `\n` vs `\r\n` is not important - assert target_path.read_text() == "somegoodstuff" + assert target_path.read_text() == "somemoregoodstuff" @pytest.fixture From f9d7d117f8ed071b187ef8f6b43fe25b5b36f104 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 17 Jan 2024 10:56:42 +0000 Subject: [PATCH 2/4] Migrate test to functional Signed-off-by: Pedro Algarvio --- .../states/file/test_accumulated.py | 81 ++++++++++++++++ tests/pytests/integration/states/test_file.py | 94 ------------------- 2 files changed, 81 insertions(+), 94 deletions(-) diff --git a/tests/pytests/functional/states/file/test_accumulated.py b/tests/pytests/functional/states/file/test_accumulated.py index f48043050b6..85e696c5630 100644 --- a/tests/pytests/functional/states/file/test_accumulated.py +++ b/tests/pytests/functional/states/file/test_accumulated.py @@ -125,3 +125,84 @@ def test_issue_11003_immutable_lazy_proxy_sum(modules, tmp_path, state_tree): for item in ("", "bar", "baz"): block_contents.remove(item) assert block_contents == [] + + +def test_issue_60426(modules, tmp_path, state_tree): + target_path = tmp_path.joinpath("etc/foo/bar") + jinja_name = "foo_bar" + jinja_contents = ( + "{% for item in accumulator['accumulated configstuff'] %}{{ item }}{% endfor %}" + ) + + sls_name = "issue-60426" + sls_contents = f""" + configuration file: + file.managed: + - name: {target_path} + - source: salt://foo_bar.jinja + - template: jinja + - makedirs: True + + accumulated configstuff: + file.accumulated: + - filename: {target_path} + - text: + - some + - good + - stuff + - watch_in: + - configuration file + """ + + sls_tempfile = pytest.helpers.temp_file( + f"{sls_name}.sls", directory=state_tree, contents=sls_contents + ) + + jinja_tempfile = pytest.helpers.temp_file( + f"{jinja_name}.jinja", directory=state_tree, contents=jinja_contents + ) + + with sls_tempfile, jinja_tempfile: + ret = modules.state.apply(sls_name) + for state_run in ret: + assert state_run.result is True + # Check to make sure the file was created + assert target_path.is_file() + # The type of new line, ie, `\n` vs `\r\n` is not important + assert target_path.read_text() == "somegoodstuff" + + sls_contents = f""" + configuration file: + file.managed: + - name: {target_path} + - source: salt://foo_bar.jinja + - template: jinja + - makedirs: True + + accumulated configstuff: + file.accumulated: + - filename: {target_path} + - text: + - some + - more + - good + - stuff + - watch_in: + - file: configuration file + """ + + sls_tempfile = pytest.helpers.temp_file( + f"{sls_name}.sls", directory=state_tree, contents=sls_contents + ) + jinja_tempfile = pytest.helpers.temp_file( + f"{jinja_name}.jinja", directory=state_tree, contents=jinja_contents + ) + + with sls_tempfile, jinja_tempfile: + ret = modules.state.apply(sls_name) + for state_run in ret: + assert state_run.result is True + # Check to make sure the file was created + assert target_path.is_file() + # The type of new line, ie, `\n` vs `\r\n` is not important + assert target_path.read_text() == "somemoregoodstuff" diff --git a/tests/pytests/integration/states/test_file.py b/tests/pytests/integration/states/test_file.py index afc3d721c0c..42bdc06baad 100644 --- a/tests/pytests/integration/states/test_file.py +++ b/tests/pytests/integration/states/test_file.py @@ -384,100 +384,6 @@ def test_issue_50221( assert target_path.read_text().replace("\r\n", "\n") == expected_content -def test_issue_60426( - salt_master, - salt_call_cli, - tmp_path, -): - target_path = tmp_path.joinpath("etc/foo/bar") - jinja_name = "foo_bar" - jinja_contents = ( - "{% for item in accumulator['accumulated configstuff'] %}{{ item }}{% endfor %}" - ) - - sls_name = "issue-60426" - sls_contents = """ - configuration file: - file.managed: - - name: {target_path} - - source: salt://foo_bar.jinja - - template: jinja - - makedirs: True - - accumulated configstuff: - file.accumulated: - - filename: {target_path} - - text: - - some - - good - - stuff - - watch_in: - - configuration file - """.format( - target_path=target_path - ) - - sls_tempfile = salt_master.state_tree.base.temp_file( - "{}.sls".format(sls_name), sls_contents - ) - - jinja_tempfile = salt_master.state_tree.base.temp_file( - "{}.jinja".format(jinja_name), jinja_contents - ) - - with sls_tempfile, jinja_tempfile: - ret = salt_call_cli.run("state.apply", sls_name) - assert ret.returncode == 0 - assert ret.data - state_run = next(iter(ret.data.values())) - assert state_run["result"] is True - # Check to make sure the file was created - assert target_path.is_file() - # The type of new line, ie, `\n` vs `\r\n` is not important - assert target_path.read_text() == "somegoodstuff" - - sls_name = "issue-60426" - sls_contents = """ - configuration file: - file.managed: - - name: {target_path} - - source: salt://foo_bar.jinja - - template: jinja - - makedirs: True - - accumulated configstuff: - file.accumulated: - - filename: {target_path} - - text: - - some - - more - - good - - stuff - - watch_in: - - file: configuration file - """.format( - target_path=target_path - ) - - sls_tempfile = salt_master.state_tree.base.temp_file( - "{}.sls".format(sls_name), sls_contents - ) - jinja_tempfile = salt_master.state_tree.base.temp_file( - "{}.jinja".format(jinja_name), jinja_contents - ) - - with sls_tempfile, jinja_tempfile: - ret = salt_call_cli.run("state.apply", sls_name) - assert ret.returncode == 0 - assert ret.data - state_run = next(iter(ret.data.values())) - assert state_run["result"] is True - # Check to make sure the file was created - assert target_path.is_file() - # The type of new line, ie, `\n` vs `\r\n` is not important - assert target_path.read_text() == "somemoregoodstuff" - - @pytest.fixture def _check_min_patch_version(shell): min_patch_ver = "2.6" From a1a7dad84d36b6599d9d8959fda11e005b7d0852 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 17 Jan 2024 13:37:06 +0000 Subject: [PATCH 3/4] Increase concurrency in CI pipelines. Signed-off-by: Pedro Algarvio --- tools/ci.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/ci.py b/tools/ci.py index 6b3a7e7064e..9e71885a8e7 100644 --- a/tools/ci.py +++ b/tools/ci.py @@ -656,7 +656,7 @@ def matrix( "functional": 3, "integration": 5, "scenarios": 1, - "unit": 2, + "unit": 4, } # On nightly and scheduled builds we don't want splits at all if workflow.lower() in ("nightly", "scheduled"): From 68131ce7ab248255feb50436a8b1c6c44813c762 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 17 Jan 2024 14:02:38 +0000 Subject: [PATCH 4/4] When running changed tests, always run all changed files tests Signed-off-by: Pedro Algarvio --- .github/workflows/test-action-linux.yml | 24 ++------------ .github/workflows/test-action-macos.yml | 40 ++--------------------- .github/workflows/test-action-windows.yml | 22 ++----------- 3 files changed, 7 insertions(+), 79 deletions(-) diff --git a/.github/workflows/test-action-linux.yml b/.github/workflows/test-action-linux.yml index bc1c0d7f218..2e4e4392931 100644 --- a/.github/workflows/test-action-linux.yml +++ b/.github/workflows/test-action-linux.yml @@ -206,31 +206,13 @@ jobs: --nox-session=${{ inputs.nox-session }} ${{ inputs.distro-slug }} \ ${{ matrix.tests-chunk }} - - name: Run Fast/Changed Tests + - name: Run Changed Tests id: run-fast-changed-tests - if: ${{ fromJSON(inputs.testrun)['type'] != 'full' && fromJSON(inputs.testrun)['selected_tests']['fast'] == false }} + if: ${{ fromJSON(inputs.testrun)['type'] != 'full' }} run: | tools --timestamps --no-output-timeout-secs=1800 --timeout-secs=14400 vm test --skip-requirements-install \ --nox-session=${{ inputs.nox-session }} --rerun-failures -E SALT_TRANSPORT ${{ matrix.fips && '--fips ' || '' }}${{ inputs.distro-slug }} \ - ${{ matrix.tests-chunk }} -- --suppress-no-test-exit-code \ - --from-filenames=testrun-changed-files.txt - - - name: Run Slow/Changed Tests - id: run-slow-changed-tests - if: ${{ fromJSON(inputs.testrun)['type'] != 'full' && fromJSON(inputs.testrun)['selected_tests']['slow'] == false }} - run: | - tools --timestamps --no-output-timeout-secs=1800 --timeout-secs=14400 vm test --skip-requirements-install \ - --nox-session=${{ inputs.nox-session }} --rerun-failures -E SALT_TRANSPORT ${{ matrix.fips && '--fips ' || '' }}${{ inputs.distro-slug }} \ - ${{ matrix.tests-chunk }} -- --no-fast-tests --slow-tests --suppress-no-test-exit-code \ - --from-filenames=testrun-changed-files.txt - - - name: Run Core/Changed Tests - id: run-core-changed-tests - if: ${{ fromJSON(inputs.testrun)['type'] != 'full' && fromJSON(inputs.testrun)['selected_tests']['core'] == false }} - run: | - tools --timestamps --no-output-timeout-secs=1800 --timeout-secs=14400 vm test --skip-requirements-install \ - --nox-session=${{ inputs.nox-session }} --rerun-failures -E SALT_TRANSPORT ${{ matrix.fips && '--fips ' || '' }}${{ inputs.distro-slug }} \ - ${{ matrix.tests-chunk }} -- --no-fast-tests --core-tests --suppress-no-test-exit-code \ + ${{ matrix.tests-chunk }} -- --core-tests --slow-tests --suppress-no-test-exit-code \ --from-filenames=testrun-changed-files.txt - name: Run Fast Tests diff --git a/.github/workflows/test-action-macos.yml b/.github/workflows/test-action-macos.yml index a6d6bbf9ca2..8f5eb111bfb 100644 --- a/.github/workflows/test-action-macos.yml +++ b/.github/workflows/test-action-macos.yml @@ -184,7 +184,7 @@ jobs: run: | sudo -E nox --force-color -e ${{ inputs.nox-session }} -- ${{ matrix.tests-chunk }} -- -k "mac or darwin" - - name: Run Fast/Changed Tests + - name: Run Changed Tests id: run-fast-changed-tests if: ${{ fromJSON(inputs.testrun)['type'] != 'full' && fromJSON(inputs.testrun)['selected_tests']['fast'] == false }} env: @@ -199,43 +199,7 @@ jobs: COVERAGE_CONTEXT: ${{ inputs.distro-slug }} run: | sudo -E nox --force-color -e ${{ inputs.nox-session }} -- ${{ matrix.tests-chunk }} -- \ - -k "mac or darwin" --suppress-no-test-exit-code \ - --from-filenames=testrun-changed-files.txt - - - name: Run Slow/Changed Tests - id: run-slow-changed-tests - if: ${{ fromJSON(inputs.testrun)['type'] != 'full' && fromJSON(inputs.testrun)['selected_tests']['slow'] == false }} - env: - SKIP_REQUIREMENTS_INSTALL: "1" - PRINT_TEST_SELECTION: "0" - PRINT_TEST_PLAN_ONLY: "0" - PRINT_SYSTEM_INFO: "0" - RERUN_FAILURES: "1" - GITHUB_ACTIONS_PIPELINE: "1" - SKIP_INITIAL_GH_ACTIONS_FAILURES: "1" - SKIP_CODE_COVERAGE: "${{ inputs.skip-code-coverage && '1' || '0' }}" - COVERAGE_CONTEXT: ${{ inputs.distro-slug }} - run: | - sudo -E nox --force-color -e ${{ inputs.nox-session }} -- ${{ matrix.tests-chunk }} -- \ - -k "mac or darwin" --suppress-no-test-exit-code --no-fast-tests --slow-tests \ - --from-filenames=testrun-changed-files.txt - - - name: Run Core/Changed Tests - id: run-core-changed-tests - if: ${{ fromJSON(inputs.testrun)['type'] != 'full' && fromJSON(inputs.testrun)['selected_tests']['core'] == false }} - env: - SKIP_REQUIREMENTS_INSTALL: "1" - PRINT_TEST_SELECTION: "0" - PRINT_TEST_PLAN_ONLY: "0" - PRINT_SYSTEM_INFO: "0" - RERUN_FAILURES: "1" - GITHUB_ACTIONS_PIPELINE: "1" - SKIP_INITIAL_GH_ACTIONS_FAILURES: "1" - SKIP_CODE_COVERAGE: "${{ inputs.skip-code-coverage && '1' || '0' }}" - COVERAGE_CONTEXT: ${{ inputs.distro-slug }} - run: | - sudo -E nox --force-color -e ${{ inputs.nox-session }} -- ${{ matrix.tests-chunk }} -- \ - -k "mac or darwin" --suppress-no-test-exit-code --no-fast-tests --core-tests \ + -k "mac or darwin" --core-tests --slow-tests --suppress-no-test-exit-code \ --from-filenames=testrun-changed-files.txt - name: Run Fast Tests diff --git a/.github/workflows/test-action-windows.yml b/.github/workflows/test-action-windows.yml index bdf11a29abd..b251a5ffce4 100644 --- a/.github/workflows/test-action-windows.yml +++ b/.github/workflows/test-action-windows.yml @@ -206,31 +206,13 @@ jobs: --nox-session=${{ inputs.nox-session }} ${{ inputs.distro-slug }} \ ${{ matrix.tests-chunk }} - - name: Run Fast/Changed Tests + - name: Run Changed Tests id: run-fast-changed-tests if: ${{ fromJSON(inputs.testrun)['type'] != 'full' && fromJSON(inputs.testrun)['selected_tests']['fast'] == false }} run: | tools --timestamps --no-output-timeout-secs=1800 --timeout-secs=14400 vm test --skip-requirements-install \ --nox-session=${{ inputs.nox-session }} --rerun-failures -E SALT_TRANSPORT ${{ matrix.fips && '--fips ' || '' }}${{ inputs.distro-slug }} \ - ${{ matrix.tests-chunk }} -- --suppress-no-test-exit-code \ - --from-filenames=testrun-changed-files.txt - - - name: Run Slow/Changed Tests - id: run-slow-changed-tests - if: ${{ fromJSON(inputs.testrun)['type'] != 'full' && fromJSON(inputs.testrun)['selected_tests']['slow'] == false }} - run: | - tools --timestamps --no-output-timeout-secs=1800 --timeout-secs=14400 vm test --skip-requirements-install \ - --nox-session=${{ inputs.nox-session }} --rerun-failures -E SALT_TRANSPORT ${{ matrix.fips && '--fips ' || '' }}${{ inputs.distro-slug }} \ - ${{ matrix.tests-chunk }} -- --no-fast-tests --slow-tests --suppress-no-test-exit-code \ - --from-filenames=testrun-changed-files.txt - - - name: Run Core/Changed Tests - id: run-core-changed-tests - if: ${{ fromJSON(inputs.testrun)['type'] != 'full' && fromJSON(inputs.testrun)['selected_tests']['core'] == false }} - run: | - tools --timestamps --no-output-timeout-secs=1800 --timeout-secs=14400 vm test --skip-requirements-install \ - --nox-session=${{ inputs.nox-session }} --rerun-failures -E SALT_TRANSPORT ${{ matrix.fips && '--fips ' || '' }}${{ inputs.distro-slug }} \ - ${{ matrix.tests-chunk }} -- --no-fast-tests --core-tests --suppress-no-test-exit-code \ + ${{ matrix.tests-chunk }} -- --core-tests --slow-tests --suppress-no-test-exit-code \ --from-filenames=testrun-changed-files.txt - name: Run Fast Tests