Skip to content

Conversation

@Roshan-R
Copy link
Contributor

No description provided.

@Roshan-R
Copy link
Contributor Author

/test

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new integration test scenario for testing-devel. The changes are straightforward, adding a new Kustomization resource and the corresponding IntegrationTestScenario definition. My feedback focuses on clarifying the intended optional status of the test and addressing hardcoded version numbers to improve maintainability and prevent potential issues with outdated test configurations.

Comment on lines +7 to +8
# Remove this label if testing is not optional.
# test.appstudio.openshift.io/optional: "true"

Choose a reason for hiding this comment

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

medium

The label test.appstudio.openshift.io/optional: "true" is currently commented out. If this integration test is intended to be optional, please uncomment this label. If it's meant to be a mandatory test, then the comment on line 7 should be removed to avoid confusion.

    test.appstudio.openshift.io/optional: "true"

- name: url
value: https://gitlab.com/testing-farm/integrations-konflux
- name: revision
value: v3.16

Choose a reason for hiding this comment

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

medium

The revision for the resolverRef is hardcoded to v3.16. The introductory comment on line 2 suggests setting the pipeline_tag to the "latest available tag". Hardcoding specific versions can lead to outdated tests if the upstream repository updates. Consider updating this value to the current latest tag or implementing a mechanism to fetch the latest version dynamically. If v3.16 is intentionally fixed for this specific test, please add a comment to clarify this.

      value: v3.16 # Fixed version for this test scenario.

value: RHEL-9-Nightly
- name: ARCH
value: x86_64
- name: IMAGE_TAG

Choose a reason for hiding this comment

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

medium

Similar to the revision field, the IMAGE_TAG parameter is also hardcoded to v3.16. Please ensure this value is kept up-to-date with the latest available tag, or add a clarifying comment if v3.16 is a fixed version for this test.

    value: v3.16 # Fixed version for this test scenario.

@Roshan-R
Copy link
Contributor Author

/retest

1 similar comment
@Roshan-R
Copy link
Contributor Author

/retest

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.

1 participant