Skip to content

[RAPTOR-15429] Skip Functional Tests for Fork PRs and Guide Contributors#373

Merged
reza-mahdiani-m merged 3 commits intodatarobot-oss:masterfrom
reza-mahdiani-m:improve_ci
Dec 30, 2025
Merged

[RAPTOR-15429] Skip Functional Tests for Fork PRs and Guide Contributors#373
reza-mahdiani-m merged 3 commits intodatarobot-oss:masterfrom
reza-mahdiani-m:improve_ci

Conversation

@reza-mahdiani-m
Copy link
Collaborator

@reza-mahdiani-m reza-mahdiani-m commented Dec 29, 2025

Description

Functional test need secrets and for security reasons secrets can not be shared between fork and the main repository.
In addition, we can not use the secrets of the main repository for testing the fork as it has security issues.
The logic of CI is changed as the following:
If the PR is in the main repo, main repo secrets would be used for running the functional test.
If PR is in the fork, function test would run on fork using user secrets and will be skipped in the main repo.

https://datarobot.atlassian.net/browse/RAPTOR-15429

@reza-mahdiani-m reza-mahdiani-m changed the title improve ci Skip Functional Tests for Fork PRs and Guide Contributors Dec 29, 2025
@reza-mahdiani-m reza-mahdiani-m changed the title Skip Functional Tests for Fork PRs and Guide Contributors RAPTOR-15429 Skip Functional Tests for Fork PRs and Guide Contributors Dec 29, 2025
@reza-mahdiani-m reza-mahdiani-m changed the title RAPTOR-15429 Skip Functional Tests for Fork PRs and Guide Contributors [RAPTOR-15429] Skip Functional Tests for Fork PRs and Guide Contributors Dec 29, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR modifies the GitHub Actions workflow to skip functional tests for pull requests originating from forked repositories. The change aims to prevent functional tests from running on fork PRs where repository secrets (like DATAROBOT_API_TOKEN and DATAROBOT_WEBSERVER) would not be available.

Key Changes

  • Added a condition to the run-functional-test job that prevents execution when the PR comes from a fork
  • Restructured the existing conditional logic with proper parentheses grouping for clarity

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github.event.comment.body == null ||
(github.event.issue.pull_request && contains(github.event.comment.body, '$FUNCTIONAL_TESTS='))
) &&
(github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork != true)
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

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

The condition uses != true to check if a PR is not from a fork. In GitHub Actions, when the event is not a pull_request (e.g., push events, issue_comment events), the property github.event.pull_request.head.repo.fork may be null or undefined. While null != true evaluates to true in JavaScript, this implicit behavior can be unclear and error-prone.

Consider using a more explicit check such as github.event.pull_request.head.repo.fork == false or restructure the condition to be more clear about handling non-PR events first.

Suggested change
(github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork != true)
(github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false)

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do not agree with you. because if event is not a pull request, it will be caught in the first block 'github.event_name != 'pull_request' and the condition is met.
However I apply your suggestion as I want my code be clean and explicit.

@reza-mahdiani-m reza-mahdiani-m merged commit 3679834 into datarobot-oss:master Dec 30, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants