From 6c3439bf1e18fdde14da92983586938a1814dac5 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Thu, 26 Oct 2023 15:02:21 +0800 Subject: [PATCH] Clean up workflow file, add documenting comments --- .github/workflows/backend-validation.yml | 61 ++++++++++++++++-------- mathesar/tests/api/test_table_api.py | 1 - 2 files changed, 42 insertions(+), 20 deletions(-) diff --git a/.github/workflows/backend-validation.yml b/.github/workflows/backend-validation.yml index bb9b8174ae..97f1823e69 100644 --- a/.github/workflows/backend-validation.yml +++ b/.github/workflows/backend-validation.yml @@ -7,12 +7,22 @@ name: Backend tests and linting on: [pull_request, merge_group] jobs: - pytest_required_check: + +################################################################################ +## FILE CHANGE CHECKERS ## +## ## +## These jobs check which files have changed so that we can call appropriate ## +## testing, auditing, and linting jobs. Jobs in this section should check for ## +## file changes that would indicate whether we need to run some particular ## +## test suite, then they should output 'true' if such file changes are found. ## +## ## +################################################################################ + + python_tests_required: + name: Check for file changes requiring python tests runs-on: ubuntu-latest - # Here, we capture whether any files have changed to indicate we need to run - # python tests outputs: - any_changed: ${{ steps.changed_files.outputs.any_changed }} + tests_should_run: ${{ steps.changed_files.outputs.any_changed }} steps: - uses: actions/checkout@v4 - name: Get changed files @@ -24,12 +34,11 @@ jobs: mathesar/** db/** - python_lint_required_check: + python_lint_required: + name: Check for file changes requiring python linter runs-on: ubuntu-latest - # Here, we capture whether any files have changed to indicate we need to run - # python linter outputs: - any_changed: ${{ steps.changed_files.outputs.any_changed }} + lint_should_run: ${{ steps.changed_files.outputs.any_changed }} steps: - uses: actions/checkout@v4 - name: Get changed files @@ -38,12 +47,11 @@ jobs: with: files: '**.py' - sql_tests_required_check: + sql_tests_required: + name: Check for file changes requiring SQL tests runs-on: ubuntu-latest - # Here, we capture whether any files have changed to indicate we need to run - # SQL tests outputs: - any_changed: ${{ steps.changed_files.outputs.any_changed }} + tests_should_run: ${{ steps.changed_files.outputs.any_changed }} steps: - uses: actions/checkout@v4 - name: Get changed files @@ -52,11 +60,24 @@ jobs: with: files: '**.sql' +################################################################################ +## TEST/LINT RUNNERS ## +## ## +## These jobs run tests and linters. Each job in this section should be ## +## dependent on one of the FILE CHANGE CHECKERS above, and should only run if ## +## appropriate files have changed. You can see this by using a `needs:` block ## +## to make the job dependent on the relevant file checker, and an `if:` block ## +## to ensure that the file checker returned 'true' before running the actual ## +## job. Job IDs in this section must be namespaced (so `python_tests` ## +## instead of just `tests`). ## +## ## +################################################################################ + python_tests: name: Run Python tests runs-on: ubuntu-latest - needs: pytest_required_check - if: needs.pytest_required_check.outputs.any_changed == 'true' + needs: python_tests_required + if: needs.python_tests_required.outputs.tests_should_run == 'true' strategy: matrix: pg-version: [13, 14, 15] @@ -79,7 +100,7 @@ jobs: name: Run SQL tests runs-on: ubuntu-latest needs: sql_tests_required_check - if: needs.sql_tests_required_check.outputs.any_changed == 'true' + if: needs.sql_tests_required.outputs.tests_should_run == 'true' strategy: matrix: pg-version: [13, 14, 15] @@ -87,6 +108,8 @@ jobs: - uses: actions/checkout@v4 - name: Copy env file run: cp .env.example .env + # The code is checked out under uid 1001 - reset this to 1000 for the + # container to run tests successfully - name: Fix permissions run: sudo chown -R 1000:1000 . - name: Build the test DB @@ -99,8 +122,8 @@ jobs: python_lint: name: Run Python linter runs-on: ubuntu-latest - needs: python_lint_required_check - if: needs.python_lint_required_check.outputs.any_changed == 'true' + needs: python_lint_required + if: needs.python_lint_required.outputs.lint_should_run == 'true' steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v4 @@ -116,8 +139,8 @@ jobs: vulture: name: Find unused code runs-on: ubuntu-latest - needs: python_lint_required_check - if: needs.python_lint_required_check.outputs.any_changed == 'true' + needs: python_lint_required + if: needs.python_lint_required.outputs.lint_should_run == 'true' steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v4 diff --git a/mathesar/tests/api/test_table_api.py b/mathesar/tests/api/test_table_api.py index 9ff7aafddc..18df6ca1e0 100644 --- a/mathesar/tests/api/test_table_api.py +++ b/mathesar/tests/api/test_table_api.py @@ -2083,7 +2083,6 @@ def test_create_table_using_duplicate_id_excel_data_file(client, duplicate_id_ex def test_create_table_using_null_id_excel_data_file(client, null_id_excel_data_file, schema): - # silly comment table_name = 'null_id' expt_name = get_expected_name(table_name, data_file=null_id_excel_data_file) first_row = (1, '1.0', 'John', '25')