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

[test_operator] Update var name for tempest override_scenario bools #1095

Conversation

elfiesmelfie
Copy link
Contributor

@elfiesmelfie elfiesmelfie commented Jan 31, 2024

After passing the cifmw_test_operator_tempest_include_list var to the role, the following error occurred in roles/test_operator/tasks/tempest-tests.yml': line 22

'cifmw_test_operator_tempest_tests_include_override_scenario' is undefined

Previously, the first condition was false and caused the check to fail. Now that that var is defined, the second condition is being evaluated.

The cifmw_test_operator_tempest_tests_include_override_scenario var does not have a default set in default/main.yml

However, defaults/main.yml defines:
cifmw_test_operator_tests_include_override_scenario: false cifmw_test_operator_tests_exclude_override_scenario: false

The names of the vars were updated so that this bool is defined.

As a pull request owner and reviewers, I checked that:

  • Appropriate testing is done and actually running
  • Appropriate documentation exists and/or is up-to-date:
    • README in the role
    • Content of the docs/source is reflecting the changes

@elfiesmelfie
Copy link
Contributor Author

elfiesmelfie commented Jan 31, 2024

For testing:

This is being tested in [1].
This commit is also part of [2], which is a dependency of [1]

For docs:
The docs did not need to be updated, they already matched what was in the task.
It was only the defaults that needed to be set.

[1] openstack-k8s-operators/telemetry-operator#284
[2] #1065

After passing the cifmw_test_operator_tempest_include_list var to the
role, the following error occurred in roles/test_operator/tasks/tempest-tests.yml': line 22

'cifmw_test_operator_tempest_tests_include_override_scenario' is undefined

Previously, the first condition was false and caused the check to fail.
Now that that var is defined, the second condition is being evaluated.

The cifmw_test_operator_tempest_tests_include_override_scenario var does not have a
default set in default/main.yml

However, defaults/main.yml defines:
cifmw_test_operator_tests_include_override_scenario: false
cifmw_test_operator_tests_exclude_override_scenario: false

The names of the vars were updated so that this bool is defined.
Copy link
Contributor

@lpiwowar lpiwowar left a comment

Choose a reason for hiding this comment

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

Thanks a lot! LGTM:)

Copy link
Contributor

@kopecmartin kopecmartin left a comment

Choose a reason for hiding this comment

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

lgtm, thanks

@raukadah
Copy link
Contributor

raukadah commented Feb 6, 2024

/approve

Copy link
Contributor

openshift-ci bot commented Feb 6, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: raukadah

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved label Feb 6, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit fa2fcee into openstack-k8s-operators:main Feb 6, 2024
7 checks passed
@elfiesmelfie elfiesmelfie deleted the efoley-test-operator-fix-var-names branch April 17, 2024 17:22
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.

4 participants