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

Integration of basic functional test suite developed with FTR into the Create Environment workflow #2219

Merged
merged 17 commits into from
May 28, 2024

Conversation

gurevichdmitry
Copy link
Collaborator

@gurevichdmitry gurevichdmitry commented May 20, 2024

Summary of your changes

This PR adds the ability to run test suites written in the Kibana repository using the Functional Test Runner (FTR). The integration is achieved through the Create Environment workflow. When the UI sanity check option is selected, the Create Environment workflow will execute UI sanity checks from the Kibana repository. The main branch of Kibana will be used for checkout.

Running Kibana tests using FTR was implemented as a separate GitHub action kibana-ftr, which is reused in the Create Environment workflow. This action checks out the specified Kibana branch without history, then installs necessary tools and performs a bootstrap. After preparation, FTR is executed with the specified configuration.

Screenshot/Data

This PR introduces two input parameters for the Create Environment workflow: run-ui-sanity-tests flag, which allows you to select whether to run UI sanity tests, and ref_commit_sha, which allows you to choose the appropriate Kibana branch, tag, or commit SHA.

Screenshot 2024-05-22 at 14 42 05

New Step in the workflow
Screenshot 2024-05-22 at 14 44 25

Report Example
Screenshot 2024-05-22 at 14 45 27

Related Issues

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary README/documentation (if appropriate)

Introducing a new rule?

Copy link

mergify bot commented May 20, 2024

This pull request does not have a backport label. Could you fix it @gurevichdmitry? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit
    NOTE: backport-skip has been added to this pull request.

Copy link

github-actions bot commented May 20, 2024

📊 Allure Report - 💚 No failures were reported.

Result Count
🟥 Failed 0
🟩 Passed 359
⬜ Skipped 33

Comment on lines +36 to +39
run-ui-sanity-tests:
description: "Run UI sanity tests after provision"
default: false
type: boolean
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 created a new input parameter to provide flexibility in selecting which tests to execute. This can also be combined with a checkbox to run both integration sanity checks and UI sanity checks.

@gurevichdmitry gurevichdmitry marked this pull request as ready for review May 22, 2024 12:03
@gurevichdmitry gurevichdmitry requested a review from a team as a code owner May 22, 2024 12:03
Comment on lines 438 to 439
es_version: ${{ env.STACK_VERSION }}
ref_commit_sha: ${{ inputs.ref_commit_sha }}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Version control for Kibana UI tests has become relevant because the UI controls implementation can differ between versions. Currently, I assume that the user executing the workflow will select the correct Kibana version and branch, for example, 8.15.0-SNAPSHOT and main, or in the future 8.15.0 and v8.15.0 in Kibana.
I am considering whether we should implement logic to prevent execution if versions do not match correctly. In the current create environment workflow, we allow installing different versions, so we need to decide if we should add additional logic to enforce version consistency, or if we will simply allow the UI tests to fail if an incorrect branch/tag is provided.

@@ -45,6 +45,10 @@ Follow these steps to run the workflow:

- **`run-sanity-tests`** (**optional**): Set to `true` to run sanity tests after the environment is set up. Default: `false`

- **`run-ui-sanity-tests`** (**optional**): Set to `true` to run Kibana UI sanity tests after the environment is set up. Default: `false`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe link the tests docs here (.github/actions/kibana-ftr/README.md)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Linked to the kibana-ftr action documentation

@@ -45,6 +45,10 @@ Follow these steps to run the workflow:

- **`run-sanity-tests`** (**optional**): Set to `true` to run sanity tests after the environment is set up. Default: `false`

- **`run-ui-sanity-tests`** (**optional**): Set to `true` to run Kibana UI sanity tests after the environment is set up. Default: `false`

- **`ref_commit_sha`** (**optional**): Specifies the Kibana branch, tag, or commit SHA to check out for the UI sanity tests, which will be executed after the environment is set up. This should correspond to the version of the `elk-stack-version` provisioned by this workflow. Note that for the current version in development, use Kibana's `main` 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.

field name is too generic IMO, consider making it more clear that it's Kibana's commit, e.g. kbn_ref_commit_sha, also since it's not referring only the commit sha, but also the branch name, maybe more generic on that side, so kibana_ref

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed to kibana_ref

export CSPM_PUBLIC_IP=$(terraform output -raw ec2_cspm_public_ip)
echo "::add-mask::$CSPM_PUBLIC_IP"
echo "CSPM_PUBLIC_IP=$CSPM_PUBLIC_IP" >> $GITHUB_ENV
run: ../../.ci/scripts/set_environment_output.sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a generic script that will be used in other workflows? If not, I would use a less generic name

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

renamed to set_cloud_env_params

@@ -45,6 +45,10 @@ Follow these steps to run the workflow:

- **`run-sanity-tests`** (**optional**): Set to `true` to run sanity tests after the environment is set up. Default: `false`

- **`run-ui-sanity-tests`** (**optional**): Set to `true` to run Kibana UI sanity tests after the environment is set up. Default: `false`

- **`ref_commit_sha`** (**optional**): Specifies the Kibana branch, tag, or commit SHA to check out for the UI sanity tests, which will be executed after the environment is set up. This should correspond to the version of the `elk-stack-version` provisioned by this workflow. Note that for the current version in development, use Kibana's `main` 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.

Can you add example supported values for each option? Just to make sure the format is clear for users

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added examples for branch, tag and commit SHA.

ref_commit_sha: ${{ github.event.inputs.ref_commit_sha || 'main' }}
test_kibana_url: ${{ secrets.TEST_KIBANA_URL }} # required
test_es_url: ${{ secrets.TEST_ES_URL }} # required
es_version: 8.15.0-SNAPSHOT # required
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this hard-coded? Or is it only for the example?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

es_version in the README is just an example.

test_kibana_url: ${{ secrets.TEST_KIBANA_URL }} # required
test_es_url: ${{ secrets.TEST_ES_URL }} # required
es_version: 8.15.0-SNAPSHOT # required
test_browser_headless: '1'
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are we using '1' as a boolean?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This originates from the FTR runner, where setting the environment variable to '1' activates headless mode. However, as per @orouz's suggestion, I'll eliminate this input variable. Since all tests are executed in headless mode on the CI, no further adjustments are necessary.

.github/actions/kibana-ftr/action.yml Outdated Show resolved Hide resolved
.github/actions/kibana-ftr/action.yml Outdated Show resolved Hide resolved
.github/actions/kibana-ftr/action.yml Outdated Show resolved Hide resolved
.github/actions/kibana-ftr/action.yml Outdated Show resolved Hide resolved
.github/actions/kibana-ftr/action.yml Outdated Show resolved Hide resolved
.github/actions/kibana-ftr/action.yml Show resolved Hide resolved
@gurevichdmitry
Copy link
Collaborator Author

This is the latest run after addressing review comments.

@gurevichdmitry gurevichdmitry merged commit 443cbdb into main May 28, 2024
25 checks passed
@gurevichdmitry gurevichdmitry deleted the dg-ftr-integration branch May 28, 2024 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants