Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MNT] Automated generation of result examples #434

Open
wants to merge 34 commits into
base: main
Choose a base branch
from
Open

Conversation

rmanaem
Copy link
Contributor

@rmanaem rmanaem commented Jan 10, 2025

Changes proposed in this pull request:

  • Implemented e2e test that mocks a successful fapi response to get a mix of aggregated and non aggregated responses in the result files
  • Implemented an action to run the e2e test and save the files as artifacts
    • Included a job in the same action to download the artifacts and use them to open a pull request against the neurobagel_examples repo

NOTE: If this pull request is to be released, the release label must be applied once the review process is done to avoid the local and remote from going out of sync as a consequence of the bump version workflow run

Checklist

This section is for the PR reviewer

  • PR has an interpretable title with a prefix ([ENH], [FIX], [REF], [TST], [CI], [MNT], [INF], [MODEL], [DOC]) (see our Contributing Guidelines for more info)
  • PR has a label for the release changelog or skip-release (to be applied by maintainers only)
  • PR links to GitHub issue with mention Closes #XXXX
  • Tests pass
  • Checks pass
  • If the PR changes the participant-level and/or the dataset-level result file, the query-tool-results files in the neurobagel_examples repo have been regenerated

For new features:

  • Tests have been added

For bug fixes:

  • There is at least one test that would fail under the original bug conditions.

Summary by Sourcery

Add a workflow to automatically update the query-tool-results example files in the neurobagel_examples repository.

CI:

  • Add a new GitHub Actions workflow to generate example result files using end-to-end tests and create a pull request to update them in the neurobagel_examples repository.

Tests:

  • Add an end-to-end test to generate example result files by mocking a successful fapi response.

Copy link

sourcery-ai bot commented Jan 10, 2025

Reviewer's Guide by Sourcery

This pull request introduces a new GitHub Actions workflow to automatically update the query-tool-results files in the neurobagel_examples repository. This is achieved by running an end-to-end test that mocks a successful fapi response, generating a mix of aggregated and non-aggregated responses. These responses are then saved as artifacts and used to create a pull request against the neurobagel_examples repository.

Sequence diagram for automated example generation workflow

sequenceDiagram
    participant GH as GitHub Actions
    participant CT as Cypress Tests
    participant API as Mock FAPI
    participant Art as Artifacts
    participant Ex as neurobagel_examples repo

    GH->>CT: Run e2e tests
    CT->>API: Mock API request
    API-->>CT: Return mock response
    CT->>Art: Save result files as artifacts
    GH->>Art: Download artifacts
    GH->>Ex: Create PR with updated files
Loading

File-Level Changes

Change Details Files
Implemented an end-to-end test and GitHub Actions workflow to update result examples
  • Created a new GitHub Actions workflow file .github/workflows/updated_examples.yaml to automate the process of updating the example result files.
  • Added an end-to-end test cypress/e2e/UpdateExamples.cy.ts to generate the example result files by mocking a successful fapi response.
  • Included logic to save the generated result files as artifacts and then use them to open a pull request against the neurobagel_examples repository.
  • Added neurobagel_examples and recipes to store the generated example result files and the configuration for generating them, respectively.
.github/workflows/updated_examples.yaml
cypress/e2e/UpdateExamples.cy.ts
neurobagel_examples
recipes
Updated existing workflows to checkout submodules
  • Added submodules: true to the checkout step in .github/workflows/component-test.yaml and .github/workflows/e2e-test.yaml to ensure that submodules are checked out during the workflow run.
.github/workflows/component-test.yaml
.github/workflows/e2e-test.yaml
Updated Cypress configuration
  • Added specPattern property to the e2e and component sections of the Cypress configuration file cypress.config.ts to specify the location of the test files.
cypress.config.ts

Assessment against linked issues

Issue Objective Addressed Explanation
#368 Ensure neurobagel_examples submodule exists in query tool repo
#368 Repurpose f-API success response example to mock the f-API response for auto-generating query result TSVs
#368 Create workflow to regenerate query result TSVs that runs when f-API example outputs are updated or query result TSV format changes

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@rmanaem rmanaem added the pr-internal Non-user-facing code improvement, will increment patch version when merged (0.0.+1) label Jan 10, 2025
Copy link

netlify bot commented Jan 10, 2025

Deploy Preview for neurobagel-query ready!

Name Link
🔨 Latest commit f319b21
🔍 Latest deploy log https://app.netlify.com/sites/neurobagel-query/deploys/678198e24ca78a000896ce77
😎 Deploy Preview https://deploy-preview-434--neurobagel-query.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

run: npm install && npm run build

- name: Run end to end tests
uses: cypress-io/github-action@v6

Check warning

Code scanning / CodeQL

Unpinned tag for a non-immutable Action in workflow Medium

Unpinned 3rd party Action 'update examples' step
Uses Step
uses 'cypress-io/github-action' with ref 'v6', not a pinned commit hash
.github/workflows/updated_examples.yaml Fixed Show fixed Hide fixed
.github/workflows/updated_examples.yaml Fixed Show fixed Hide fixed
@rmanaem rmanaem marked this pull request as draft January 10, 2025 20:31

- name: Create Pull Request
id: create_pr
uses: peter-evans/create-pull-request@v7

Check warning

Code scanning / CodeQL

Unpinned tag for a non-immutable Action in workflow Medium

Unpinned 3rd party Action 'update examples' step
Uses Step: create_pr
uses 'peter-evans/create-pull-request' with ref 'v7', not a pinned commit hash
@rmanaem rmanaem marked this pull request as ready for review January 10, 2025 21:54
Comment on lines +9 to +42
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4
with:
submodules: true

- name: Set up node env
uses: actions/setup-node@v4
with:
node-version: 20

- name: Create .env file
run: |
echo -e "NB_API_QUERY_URL=https://federate.neurobagel.org/\nNB_ENABLE_AUTH=true\nNB_QUERY_CLIENT_ID=mockclientid" > .env
- name: build
run: npm install && npm run build

- name: Run end to end tests
uses: cypress-io/github-action@v6
with:
wait-on: http://localhost:5173
start: npm run preview
spec: cypress/e2e/UpdateExamples.cy.ts
component: false

- name: Upload test artifacts
uses: actions/upload-artifact@v4
with:
name: query-tool-results
path: cypress/downloads/*

update-query-tool-results:

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions Job or Workflow does not set permissions
Comment on lines +43 to +77
runs-on: ubuntu-latest
needs: generate-example-files
steps:
- name: Generate a token
id: generate-token
uses: actions/create-github-app-token@v1
with:
app-id: ${{ vars.NB_BOT_ID }}
private-key: ${{ secrets.NB_BOT_KEY }}
owner: ${{ github.repository_owner }}

- name: Checkout neurobagel_examples repository
uses: actions/checkout@v4
with:
repository: neurobagel/neurobagel_examples
token: ${{ steps.generate-token.outputs.token }}

- name: Download artifacts
uses: actions/download-artifact@v4
with:
name: query-tool-results
path: neurobagel_examples/query-tool-results

- name: Create Pull Request
id: create_pr
uses: peter-evans/create-pull-request@v7
with:
token: ${{ steps.generate-token.outputs.token }}
commit-message: Update `query-tool-results` files
title: Update `query-tool-results` files
body: "This PR updates the `query-tool-results` files with the latest changes."
base: main
branch: update-query-tool-results
committer: NeuroBagel Bot <neurobagel-bot[bot]@users.noreply.github.com>
labels: _bot

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions Job or Workflow does not set permissions
Copy link
Contributor

@surchs surchs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rmanaem for fixing this! Very cool workflow.

I left one idea comment, and one request to change before merging (about comment in the test). Take a look, but I'll approve now already

🧑‍🍳

.github/workflows/component-test.yaml Show resolved Hide resolved
Comment on lines +8 to +40
generate-example-files:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4
with:
submodules: true

- name: Set up node env
uses: actions/setup-node@v4
with:
node-version: 20

- name: Create .env file
run: |
echo -e "NB_API_QUERY_URL=https://federate.neurobagel.org/\nNB_ENABLE_AUTH=true\nNB_QUERY_CLIENT_ID=mockclientid" > .env
- name: build
run: npm install && npm run build

- name: Run end to end tests
uses: cypress-io/github-action@v6
with:
wait-on: http://localhost:5173
start: npm run preview
spec: cypress/e2e/UpdateExamples.cy.ts
component: false

- name: Upload test artifacts
uses: actions/upload-artifact@v4
with:
name: query-tool-results
path: cypress/downloads/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could just skip this whole job and instead make the existing e2e test workflow upload this artefact. There is a nice mechanism for this available: https://github.com/github/docs/blob/main/content/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows.md#using-data-from-the-triggering-workflow

But cypress/e2e/UpdateExamples.cy.ts isn't a test, but just a way to use cypress to make the example files. So maybe it's more confusing to use the e2e test outputs. In either case, that would be a change for a future PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, the main reason I kept it separate from the other workflow is because we generate the result files with different content in other tests and if I'm not mistaken cypress overwrites them so depending on which test runs first we might end up with the wrong examples in the neurobage_examples repo.

@@ -3,11 +3,13 @@ import { defineConfig } from 'cypress';

export default defineConfig({
e2e: {
specPattern: 'cypress/e2e/*',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of interest: what does this do and why is it needed?

cypress/e2e/UpdateExamples.cy.ts Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-internal Non-user-facing code improvement, will increment patch version when merged (0.0.+1)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update auto-generation of query result examples
2 participants