[GEN-1654] Trigger integration tests via github PR label

[rxu17]

Problem:

We only have one test synapse project in genie, and often times, we run into an concurrency issue with the CI/CD integration tests (which runs the test pipeline in synapse) because our synapse project cannot handle more than 1 ongoing pipeline run.

Here are some of the situations we would get concurrency runs:

• push multiple commits within the span of an hour or so but maybe the integration tests in the test synapse project hasn't finished running from the 1st commit, so we have multiple ongoing
• multiple people testing/working on the same pipeline
• we have a scheduled automate truststore ci/cd which runs parts of the test pipeline

JIRA Ticket: https://sagebionetworks.jira.com/browse/GEN-1654

Workflow mini design doc: https://sagebionetworks.jira.com/browse/GEN-1654?focusedCommentId=267713

Solution:

Only trigger the lint, pytest, build-container and integration tests github action jobs when we have the following conditions:

• When github PR label is run_integration_tests AND the PR is open
• When there is a push to develop or main branches

The CI/CD for when a release event happens stays the same.

This gives us time to check for concurrent runs / builds / change the annotations / etc. This will change the workflow so that we have to open up at least a draft PR in order to get a container build as well for the feature branch we are developing on

---
title: GitHub Actions Build Workflow (with Conditions)
config:
  theme: "default"
---

flowchart TD
    A([Trigger Event])
    subgraph Triggers
      direction TB
      PR["Pull Request with label:<br/>'run_integration_tests'<br/>and open state"]
      PUSH["Push to main<br/>or develop"]
      REL["Release event"]
    end

    A --> PR
    A --> PUSH
    A --> REL

    subgraph Jobs Order
      direction TB
      DET["determine-changes"]
      TEST["test"]
      LINT["lint"]
      BUILD["build-container"]
      INTEG["integration-tests"]
      DEPLOY["deploy"]
    end

    %% determine-changes
    PR -. "Runs if PR has label\nand is open" .-> DET
    PUSH -. "Runs if push to main/develop" .-> DET
    REL -. "Runs on release" .-> DET

    %% test
    DET -- "Always needed (runs in parallel)" --> TEST

    %% lint
    DET -- "Always needed (runs in parallel)" --> LINT

    %% build-container
    TEST -.-> BUILD
    LINT -.-> BUILD
    BUILD -. "Always, except PR events with incomplete jobs" .-> BUILD

    %% integration-tests
    BUILD -.-> INTEG
    LINT -.-> INTEG
    TEST -.-> INTEG
    DET -. "Needs changes outside tests/" .-> INTEG

    %% deploy job only for release
    BUILD -.-> DEPLOY
    LINT -.-> DEPLOY
    TEST -.-> DEPLOY
    REL -. "Only for release event" .-> DEPLOY

    %% Job notes
    INTEG:::cond_note
    DEPLOY:::cond_note

    classDef cond_note fill:#fffae1,stroke:#aaa,stroke-width:1px,color:#b06b00;
    class INTEG,DEPLOY cond_note;

    %% Legend
    subgraph Legend [Legend]
      direction LR
      note1(( )):::cond_note
      note2["Job only runs for special condition"]
    end

Loading

Testing:

• Closed the PR and pushed commits to branch - no build run happens
• Opened the PR (no github label) - no build run happens
• Added the label run_integration_tests - build occurs: https://github.com/Sage-Bionetworks/Genie/actions/runs/17963691035
• Push a commit to branch while PR is open and has the label run_integration_tests - build occurs: https://github.com/Sage-Bionetworks/Genie/actions/runs/17963996288
• Push a commit to branch while PR is open and label run_integration_testsis removed - no build, it is skipped: https://github.com/Sage-Bionetworks/Genie/actions/runs/17964020874

[danlu1]

QQ: The instruction seems not aligned with the diagram: 1. I saw integration-tests is also the downstream of lint and test. 2. deploy is the following step of test and lint.

[rxu17]

Yes those two points are accurate. I think you just meant I need to elaborate more / include out the details of the upstream and downstream points? It's true that deploy job only runs when there is a release event. I think my goal with this section was keep it to the key points and not rewrite what the mermaid diagram is doing word for word.

I think I will just include the mermaid diagram in a README section instead because it's more accurate and easier than using words to fully describe out the process because as you can see, the github workflow is kind of complex and then keep a higher level overview like the docstring here.

[danlu1]

I saw Jenny's comment on another PR to resolve concurrency issues. Do you plan to include it in the workflow?

[rxu17]

I saw Jenny's comment on another PR to resolve concurrency issues. Do you plan to include it in the workflow?

I thought about it but I don't think it's as applicable to our scenario.

We don't want to run that many rounds of integration tests for every push we make, which is why I thought applying github labels during a draft/non-draft PR to trigger the whole lint, tests, integration tests build makes more sense.

The concurrency feature also doesn't solve this scenario:

• multiple people testing/working on the same pipeline (this includes people running nextflow test pipelines on seqera and people running locally etc without github actions workflow)

And even if the concurrency is applied, the two concurrent jobs means one job will be in a queue while the other one runs. Typically for integration tests run on the test pipeline, you'd have to immediately reset the isProcessing and rename maf database after your job is done so that the queued job won't run into those issues bc from the docs, the queued job would start running immediately after your other job finishes. Generally that wouldn't be practical because usually when integration tests are running, it takes some time and we go off to do other things.

TLDR: The goal of this PR is so we can run integration tests for a feature when we see that there are no other ongoing runs of the test pipeline/docker builds/etc and choose to trigger it when we are ready (after a bunch of pushes of code for example) which is what I think github labels resolves.

[danlu1]

I saw Jenny's comment on another PR to resolve concurrency issues. Do you plan to include it in the workflow?

I thought about it but I don't think it's as applicable to our scenario.

We don't want to run that many rounds of integration tests for every push we make, which is why I thought applying github labels during a draft/non-draft PR to trigger the whole lint, tests, integration tests build makes more sense.

The concurrency feature also doesn't solve this scenario:

• multiple people testing/working on the same pipeline (this includes people running nextflow test pipelines on seqera and people running locally etc without github actions workflow)

And even if the concurrency is applied, the two concurrent jobs means one job will be in a queue while the other one runs. Typically for integration tests run on the test pipeline, you'd have to immediately reset the isProcessing and rename maf database after your job is done so that the queued job won't run into those issues bc from the docs, the queued job would start running immediately after your other job finishes. Generally that wouldn't be practical because usually when integration tests are running, it takes some time and we go off to do other things.

TLDR: The goal of this PR is so we can run integration tests for a feature when we see that there are no other ongoing runs of the test pipeline/docker builds/etc and choose to trigger it when we are ready (after a bunch of pushes of code for example) which is what I think github labels resolves.

Since the isProcessing and maf database need to reset anyway if there are needs to run multiple rounds of integration tests, maybe we could flag this somewhere, README?

[rxu17]

Since the isProcessing and maf database need to reset anyway if there are needs to run multiple rounds of integration tests, maybe we could flag this somewhere, README?

Yes my plan was to include it along with this new feature in the SOP: https://sagebionetworks.jira.com/wiki/spaces/APGD/pages/3562012688/Workflow+for+Adding+New+Features once we confirmed the workflow.

[danlu1]

Since the isProcessing and maf database need to reset anyway if there are needs to run multiple rounds of integration tests, maybe we could flag this somewhere, README?

Yes my plan was to include it along with this new feature in the SOP: https://sagebionetworks.jira.com/wiki/spaces/APGD/pages/3562012688/Workflow+for+Adding+New+Features once we confirmed the workflow.

Sounds good to me

[rxu17]

I will merge this in after the 17.0 package release

[rxu17]

This also needs to wait on the final step of the 17.0 package release - merging the main branch back into develop (to update the __version__)

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud