Tech Debt: Retry failed integration tests (#1269)

* Add support for retrying failed integration tests and uploading results

Author: BryanFauble

.github/workflows/build.yml

@@ -125,12 +125,20 @@ jobs:
             echo "synapse_pat_available=true" >> $GITHUB_OUTPUT;
           fi
 
+      - name: Download Failed Tests from Previous Attempt
+        if: ${{ github.run_attempt > 1 && (contains(fromJSON('["3.9"]'), matrix.python) || contains(fromJSON('["3.13"]'), matrix.python)) && steps.secret-check.outputs.secrets_available == 'true'}}
+        uses: actions/download-artifact@v4
+        with:
+          name: failed-tests-${{ matrix.os }}-${{ matrix.python }}
+        continue-on-error: true
+
       # run integration tests iff the decryption keys for the test configuration are available.
       # they will not be available in pull requests from forks.
       # run integration tests on the oldest and newest supported versions of python.
       # we don't run on the entire matrix to avoid a 3xN set of concurrent tests against
       # the target server where N is the number of supported python versions.
       - name: run-integration-tests
+        id: integration_tests
         shell: bash
 
         # keep versions consistent with the first and last from the strategy matrix
@@ -204,11 +212,96 @@ jobs:
             echo "Running integration tests for Max Python version (3.13) - ignoring synchronous tests"
           fi
 
-          # use loadscope to avoid issues running tests concurrently that share scoped fixtures
-          pytest -sv --reruns 3 --cov-append --cov=. --cov-report xml tests/integration -n 8 $IGNORE_FLAGS --dist loadscope
+          # Check if we should run only failed tests from previous attempt
+          if [[ -f failed_tests.txt && -s failed_tests.txt ]]; then
+            echo "::notice::Retry attempt ${{ github.run_attempt }} detected - running only previously failed tests"
+            cat failed_tests.txt
+
+            # Run only the failed tests
+            pytest -sv --reruns 3 --cov-append --cov=. --cov-report xml \
+              --junit-xml=test-results.xml \
+              -n 8 --dist loadscope \
+              $(cat failed_tests.txt | tr '\n' ' ')
+          else
+            echo "::notice::First attempt or no previous failures - running full integration test suite"
+
+            # use loadscope to avoid issues running tests concurrently that share scoped fixtures
+            pytest -sv --reruns 3 --cov-append --cov=. --cov-report xml \
+              --junit-xml=test-results.xml \
+              tests/integration -n 8 $IGNORE_FLAGS --dist loadscope
+          fi
 
           # Execute the CLI tests in a non-dist way because they were causing some test instability when being run concurrently
           pytest -sv --reruns 3 --cov-append --cov=. --cov-report xml tests/integration/synapseclient/test_command_line_client.py
+
+      - name: Extract Failed Tests
+        if: always() && steps.integration_tests.outcome == 'failure'
+        shell: bash
+        run: |
+          python -c "
+          import xml.etree.ElementTree as ET
+          import os
+          tree = ET.parse('test-results.xml')
+          root = tree.getroot()
+          failed = []
+          for testcase in root.iter('testcase'):
+              if testcase.find('failure') is not None or testcase.find('error') is not None:
+                  classname = testcase.get('classname')
+                  name = testcase.get('name')
+                  file_attr = testcase.get('file')
+
+                  # Use the file attribute if available, otherwise convert classname
+                  if file_attr:
+                      # file attribute is already in the correct format
+                      test_path = f'{file_attr}::{classname.split(\".\")[-1]}::{name}'
+                  else:
+                      # Convert classname from dot notation to file path
+                      # e.g., 'tests.integration.foo.test_bar.TestClass' -> 'tests/integration/foo/test_bar.py::TestClass'
+                      parts = classname.split('.')
+                      # Find the test file (usually starts with 'test_')
+                      module_parts = []
+                      class_parts = []
+                      found_test_file = False
+                      for part in parts:
+                          if not found_test_file:
+                              module_parts.append(part)
+                              if part.startswith('test_'):
+                                  found_test_file = True
+                          else:
+                              class_parts.append(part)
+
+                      file_path = '/'.join(module_parts) + '.py'
+                      if class_parts:
+                          test_path = f'{file_path}::{\"::\".join(class_parts)}::{name}'
+                      else:
+                          test_path = f'{file_path}::{name}'
+
+                  failed.append(test_path)
+
+          with open('failed_tests.txt', 'w') as f:
+              f.write('\n'.join(failed))
+          print(f'Found {len(failed)} failed tests')
+          for test in failed:
+              print(f'  - {test}')
+          print(f'Current attempt: ${{ github.run_attempt }}')
+          "
+
+      - name: Upload Failed Tests for Next Attempt
+        if: always() && steps.integration_tests.outcome == 'failure' && github.run_attempt < 3
+        uses: actions/upload-artifact@v4
+        with:
+          name: failed-tests-${{ matrix.os }}-${{ matrix.python }}
+          path: failed_tests.txt
+          retention-days: 2
+          overwrite: true
+
+      - name: Fail job if integration tests failed after all retries
+        if: always() && steps.integration_tests.outcome == 'failure'
+        shell: bash
+        run: |
+          echo "::error::Integration tests failed after ${{ github.run_attempt }} attempt(s)"
+          exit 1
+
       - name: Upload coverage report
         id: upload_coverage_report
         uses: actions/upload-artifact@v4

tests/integration/synapseclient/models/synchronous/test_schema_organization.py

@@ -210,9 +210,9 @@ async def test_store_and_get(self, json_schema: JSONSchema) -> None:
         assert json_schema.organization_name
         assert json_schema.uri
         assert json_schema.organization_id
-        assert json_schema.id
         assert json_schema.created_by
         assert json_schema.created_on
+        assert not json_schema.id
         # AND it should be getable by future instances with the same name
         js2 = JSONSchema(json_schema.name, json_schema.organization_name)
         js2.get(synapse_client=self.syn)