Skip to content

set DEST_DIR before fetch_tools.sh script#2721

Merged
openshift-merge-bot[bot] merged 2 commits intoopenshift:mainfrom
agullon:fix_rf_test_run
Dec 13, 2023
Merged

set DEST_DIR before fetch_tools.sh script#2721
openshift-merge-bot[bot] merged 2 commits intoopenshift:mainfrom
agullon:fix_rf_test_run

Conversation

@agullon
Copy link
Contributor

@agullon agullon commented Dec 13, 2023

Fix a bug introduced on #2647 PR: DEST_DIR must be set right before installing tools to set the proper path.

DEST_DIR specifies where to install the tool. robotframework is installed in microshift/_output/robotenv instead of microshift/microshift-rf-tests@tmp/venv. This causes robot framework execution to fail: /home/jenkins/ws/workspace/microshift/microshift-rf-tests/microshift/test/run.sh: line 104: /home/jenkins/ws/workspace/microshift/microshift-rf-tests@tmp/venv/bin/robot: No such file or directory

I found this bug last night Jenkins run: https://mastern-jenkins-csb-openshift-qe.apps.ocp-c1.prod.psi.redhat.com/job/microshift/job/microshift-rf-tests/923/console

@openshift-ci openshift-ci bot requested review from jogeo and pliurh December 13, 2023 09:59
test/run.sh Outdated
"${ROOTDIR}/scripts/fetch_tools.sh" robotframework
"${ROOTDIR}/scripts/fetch_tools.sh" yq
DEST_DIR="${RF_VENV}" "${ROOTDIR}/scripts/fetch_tools.sh" robotframework
DEST_DIR="${RF_VENV}" "${ROOTDIR}/scripts/fetch_tools.sh" yq
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it necessary to change the default DEST_DIR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because DEST_DIR specifies where to install the tool. robotframework is installed in microshift/_output/robotenv instead of microshift/microshift-rf-tests@tmp/venv. This causes robot framework execution to fail: /home/jenkins/ws/workspace/microshift/microshift-rf-tests/microshift/test/run.sh: line 104: /home/jenkins/ws/workspace/microshift/microshift-rf-tests@tmp/venv/bin/robot: No such file or directory

I found this bug last night Jenkins run: https://mastern-jenkins-csb-openshift-qe.apps.ocp-c1.prod.psi.redhat.com/job/microshift/job/microshift-rf-tests/923/console

This change PR reverts the change in the Stress Testing PR

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason there's a variable is so if there is environment-specific need it can be handled by the caller of this script. Can we make the installation destination use the default here, and export the variable in the QE script that invokes it?

Copy link
Contributor

@dhellmann dhellmann left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 13, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 13, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: agullon, dhellmann

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

The pull request process is described here

Details 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 Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 13, 2023
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD bb77636 and 2 for PR HEAD b33ec26 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 13, 2023

@agullon: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit 33f5fc2 into openshift:main Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants