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

Add integration tests for steve OR and NOT filters #40586

Merged
merged 3 commits into from
May 4, 2023

Conversation

cmurphy
Copy link
Contributor

@cmurphy cmurphy commented Feb 16, 2023

Issue: #40141

Depends on rancher/steve#76

Problem

Solution

Testing

Engineering Testing

Manual Testing

Automated Testing

QA Testing Considerations

Regressions Considerations

@cmurphy cmurphy changed the title [WIP] Add integration tests for steve OR filter Add integration tests for steve OR filter Mar 7, 2023
@cmurphy cmurphy changed the title Add integration tests for steve OR filter Add integration tests for steve OR and NOT filters Mar 7, 2023
@cmurphy cmurphy requested review from KevinJoiner and igomez06 March 30, 2023 11:52
KevinJoiner
KevinJoiner previously approved these changes Mar 31, 2023
igomez06
igomez06 previously approved these changes Mar 31, 2023
Copy link

@igomez06 igomez06 left a comment

Choose a reason for hiding this comment

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

Looks good. This specific to v2.7 correct?

@cmurphy
Copy link
Contributor Author

cmurphy commented Apr 11, 2023

@igomez06 correct, this is 2.7-only

@cmurphy cmurphy dismissed stale reviews from igomez06 and KevinJoiner via a1705f3 April 17, 2023 17:50
@cmurphy
Copy link
Contributor Author

cmurphy commented Apr 17, 2023

Rebased.

Had to add elements of #40536 here to run integration tests on local, I misinterpreted #40310 (comment) and it turns out that causes output.csv to be overwritten with an empty file, which we don't want.

@cmurphy cmurphy requested review from igomez06 and KevinJoiner April 17, 2023 17:53
igomez06
igomez06 previously approved these changes Apr 17, 2023
Copy link

@igomez06 igomez06 left a comment

Choose a reason for hiding this comment

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

Looks good. For the unwanted empty output.csv conditional, will you do a v2.6 PR?

@cmurphy
Copy link
Contributor Author

cmurphy commented Apr 17, 2023

Good point, yes can do

@cmurphy
Copy link
Contributor Author

cmurphy commented Apr 17, 2023

I split the local/downstream addition into a separate commit so I could backport it. Also added a commit to make the output testing more flexible, since @rmweir pointed out these don't work on k8s 1.23.

@cmurphy cmurphy requested a review from igomez06 April 17, 2023 20:34
igomez06
igomez06 previously approved these changes Apr 18, 2023
Copy link

@igomez06 igomez06 left a comment

Choose a reason for hiding this comment

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

looks good

KevinJoiner
KevinJoiner previously approved these changes May 4, 2023
cmurphy added 3 commits May 4, 2023 08:46
The behavior on local vs downstream can sometimes be subtly different,
so it is worth running API tests on both. Also, the steveapi tests are
self-documenting, but they were not writing their outputs because their
outputs were different from run to run because the downstream cluster ID
was different. This change ensures both sets of tests are run but the
output examples are only recorded when the output is deterministic
(which is for the local).
The steveapi integration tests try to be precise about the desired
output of API calls. This was causing a problem when run on k8s 1.23
which has default token secrets in every namespace and so was adding
extra resources to the response output. This change adds a label to the
secrets created in TestList so that they can easily be identified on a
live cluster and so unlabeled secrets can be ignored.
@cmurphy
Copy link
Contributor Author

cmurphy commented May 4, 2023

had to rebase

@cmurphy cmurphy merged commit 8e049e5 into rancher:release/v2.7 May 4, 2023
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.

3 participants