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

Adding Rootstock Integration Tests workflow to Powpeg node repo #310

Merged

Conversation

rmoreliovlabs
Copy link
Contributor

@rmoreliovlabs rmoreliovlabs commented Sep 4, 2024

Adding of Rootstock Integration Tests Workflow for PowPeg

This GitHub Actions workflow is designed to automate the execution of Rootstock Integration Tests for the PowPeg Node repository. The primary goal is to ensure that integration tests are run manually or automatically for opened pull requests against the master branch or any *-rc branches, as well as for direct pushes to these branches. There's a similar pr for this feature in the RSKJ repo, check here.

Implementation Details

Triggers:

  • Pull Requests: The workflow triggers on pull requests opened or reopened against any branch, specifically targeting master and *-rc branches.
  • Push Events: The workflow triggers on pushes to master and *-rc branches.
  • Manual Trigger: The workflow can be manually triggered using the workflow_dispatch event, which allows for additional customization via inputs.

How to Trigger the Workflow

Automatically:

  • Pull Requests: The workflow will automatically trigger when a pull request is opened or reopened against the master or *-rc branches.
  • Pushes: The workflow will automatically trigger on pushes to the master and *-rc branches.

Manually:

Workflow Dispatch: You can manually trigger the workflow using the workflow_dispatch event. To do this, follow these steps:

  • Navigate to the Actions tab in your GitHub repository.
  • Select the Rootstock Integration Tests workflow from the list of workflows.
  • Click on the Run workflow button.
  • Choose the branch from which you want to run the workflow using the "Use workflow from branch" dropdown menu. This allows you to select the branch (e.g., feature-branch) that will be used for the POWPEG_BRANCH value.
  • Enter the desired input parameters (if any) or leave them as their default values.

Inputs for Manual Trigger:

  • rit-branch: The branch for Rootstock Integration Tests.
    Default: main
  • rskj-branch: The branch for RSKJ.
    Default: master

To manually trigger the workflow using workflow_dispatch, the workflow file must be present in the master branch. That's we can't manually test this workflow until it's merged into master. For more info, check here and here.

Motivation and Context

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • Tests for the changes have been added (for bug fixes / features)
  • Requires Activation Code (Hard Fork)
  • Other information:

@rmoreliovlabs rmoreliovlabs force-pushed the feature/add_rootstock_integration_tests_workflow_for_powpeg branch from d361c38 to 215b151 Compare September 4, 2024 00:35
@rmoreliovlabs rmoreliovlabs reopened this Sep 4, 2024
@rmoreliovlabs rmoreliovlabs marked this pull request as ready for review September 4, 2024 00:49
@rmoreliovlabs rmoreliovlabs requested a review from a team as a code owner September 4, 2024 00:49
Copy link
Contributor

@fmacleal fmacleal left a comment

Choose a reason for hiding this comment

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

Great job, I think we are almost there and there a few things to change and we are done. 😃

.github/workflows/rit.yml Outdated Show resolved Hide resolved
.github/workflows/rit.yml Outdated Show resolved Hide resolved
.github/workflows/rit.yml Outdated Show resolved Hide resolved
.github/workflows/rit.yml Outdated Show resolved Hide resolved
fmacleal
fmacleal previously approved these changes Sep 8, 2024
Copy link
Contributor

@fmacleal fmacleal left a comment

Choose a reason for hiding this comment

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

Good job! I just have a question regarding your last change, but it doesn't prevent the approval. I think this way it will fill the initial requirements we have for this pipeline, we can always change if needed. :)

.github/workflows/rit.yml Show resolved Hide resolved
In order to have the RIT tests working as we have it today, we need to add triggers for
every commit and for when the PR is opened, but only for masters and *-rc branches base branches.
fmacleal
fmacleal previously approved these changes Sep 9, 2024
bernacodesido
bernacodesido previously approved these changes Sep 9, 2024
Copy link
Member

@bernacodesido bernacodesido left a comment

Choose a reason for hiding this comment

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

LGTM

.github/workflows/rit.yml Show resolved Hide resolved
Comment on lines +31 to +32
RIT_BRANCH="${{ github.event.inputs.rit-branch || 'main' }}"
RSKJ_BRANCH="${{ github.event.inputs.rskj-branch || 'master' }}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a way to somehow re-use the default branch values that are defined above?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hello, yes, it seems possible. Do you think it's better to re-use it? Nevertheless, to change these default values we would have to change the worfklow yaml anyway.

We would have to do something like this:

Suggested change
RIT_BRANCH="${{ github.event.inputs.rit-branch || 'main' }}"
RSKJ_BRANCH="${{ github.event.inputs.rskj-branch || 'master' }}"
RIT_BRANCH="${{ github.event.inputs.rit-branch || github.event.inputs['rit-branch'].default }}"
RSKJ_BRANCH="${{ github.event.inputs.rskj-branch || github.event.inputs['rskj-branch'].default }}"

Copy link
Contributor

@fmacleal fmacleal Sep 11, 2024

Choose a reason for hiding this comment

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

Ops, the suggestion failed. Probably because the inputs are define only if the pipeline is triggered via workflow_dispatch. This logic wouldn't work for the commits in PR.

Let's let as it was? Otherwise we will need an even bigger if like:

 RSKJ_BRANCH="${{ github.event.inputs.rskj-branch || github.event.inputs['rskj-branch'].default || 'main' }}"

Copy link
Collaborator

Choose a reason for hiding this comment

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

No big deal, I wouldn't expect the default branches to change anytime soon. Just the good practice of avoid repetition (DRY). But in this case I'd go with the simplest solution

@fmacleal fmacleal dismissed stale reviews from bernacodesido and themself via 78150c1 September 11, 2024 11:18
Copy link

@marcos-iov
Copy link
Collaborator

pipeline:run

@marcos-iov marcos-iov merged commit 59d724d into master Sep 11, 2024
7 of 8 checks passed
@marcos-iov marcos-iov deleted the feature/add_rootstock_integration_tests_workflow_for_powpeg branch September 11, 2024 14:58
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.

4 participants