Skip to content
This repository has been archived by the owner on Jul 25, 2022. It is now read-only.

refactor functions and add test case in miscellaneous.go & miscellaneous_test.go and others which impacted #404

Merged
merged 3 commits into from
Nov 19, 2020

Conversation

tedteng
Copy link
Contributor

@tedteng tedteng commented Oct 21, 2020

What this PR does / why we need it:
After a series of different experiments, to compatible the existing go test suits (*_test.go) and use it in the future test case. I have to refactor isTargeted,getTargetName,getSeedObject,getShootObject,`getProjectObject'

meanwhile, also add new test cases for those methods in miscellaneous_test.go as much as I can. add more local integration test to verify the refactor

Which issue(s) this PR fixes:
Fixes #403

Special notes for your reviewer:

Release note:


@tedteng tedteng requested a review from a team as a code owner October 21, 2020 06:52
@gardener-robot gardener-robot added needs/review Needs review size/s Size of pull request is small (see gardener-robot robot/bots/size.py) labels Oct 21, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 21, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 21, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 21, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 21, 2020
pkg/cmd/miscellaneous.go Show resolved Hide resolved
@gardener-robot gardener-robot added needs/changes Needs (more) changes and removed needs/review Needs review labels Oct 21, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 22, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 22, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 22, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 22, 2020
@tedteng tedteng changed the title add test case for miscellaneous add test case for miscellaneous and modify isTargeted Oct 22, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 22, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 22, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 22, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 10, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 10, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 10, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 10, 2020
@neo-liang-sap
Copy link
Contributor

according to discussion in planning mtg this PR is done, right? @dansible could you please approve this PR and i will perform merge? thanks!

@dansible
Copy link
Contributor

So far so good, I just wanted to go through a review of the changes with @tedteng in person. We will have a call at 10AM Friday your time for this - I've asked Ted to send you an invite in case you'd like to join also

@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 13, 2020
@tedteng
Copy link
Contributor Author

tedteng commented Nov 13, 2020

@dansible done adding extra tests in miscellaneous_test.go which test get project/shoot/seed object

})
})

Context("GetTargetName garden", func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a duplicate test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, just removed

@tedteng tedteng force-pushed the add_test_miscellaneous branch 2 times, most recently from bff9c6a to d1955d6 Compare November 17, 2020 01:19
@tedteng
Copy link
Contributor Author

tedteng commented Nov 17, 2020

@dansible fixed the conflict. Please let me know is anything else need to change it thanks

@dansible
Copy link
Contributor

Thanks @tedteng - there are just some conflicts in the logs.go file. Can you pull master and rebase?

@tedteng
Copy link
Contributor Author

tedteng commented Nov 18, 2020

@dansible done

@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 18, 2020
refactor methods able to use go test framework

fix nonexisting return and use PrintoutObject

add GetTargetedxxxObject

fix loop issue

add getRole targetReader interface

modify GetXXXObject

fix no kind ShootList

fix getRole == user
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 18, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs/changes Needs (more) changes needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/second-opinion Needs second review by someone else size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor functions and add test case in miscellaneous.go and miscellaneous_test.go and others which impacted
9 participants