Clean up workflow file, add documenting comments

mathesar-foundation · Oct 26, 2023 · 6c3439b · 6c3439b
1 parent b5806d4
commit 6c3439b
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 20 deletions.
diff --git 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,14 +100,16 @@ 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]
     steps:
       - 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
@@ -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')