Skip to content

Conversation

@AlexNPavel
Copy link
Contributor

Description of the change: Add proposal for a YAML Defined Tests plugin for the scorecard

Motivation for the change: This was initially proposed in PR #1049, but has since been split off into this PR as the goals of that proposal changed. There is a lot of discussion over this design on that PR.

This PR will initially be created as a draft as I have not yet implemented various improvements that I've planned. It will be changed to a standard PR once I make those changes.

This PR also depends on PR #1049 getting merged first, and it may be wise to wait for the implementation of that proposal before fully moving forward with this PR.

@AlexNPavel AlexNPavel added kind/feature Categorizes issue or PR as related to a new feature. scorecard Issue relates to the scorecard subcomponent labels Mar 25, 2019
@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 25, 2019
@AlexNPavel AlexNPavel force-pushed the scorecard-usertest-proposal-2 branch 2 times, most recently from 27f76e9 to 697c985 Compare May 28, 2019 23:39
@AlexNPavel
Copy link
Contributor Author

This is ready for review.

@AlexNPavel AlexNPavel marked this pull request as ready for review May 28, 2019 23:40
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 28, 2019
For `Status` and `Resources` fields, we can implement a bit of extra computation instead of simple string checking. For instance,
in the memcached-operator test, we should expect that the length of the `nodes` field (which is an array) has a certain length. To implement functions like
these, we can create some functions for these checks that are prepended by `scorecard_function_` and take an array of objects. For instance, in the above
example, `scorecard_function_length` would check that each field listed under it matches the specified length (like `nodes: 4`). If the yaml key does not
Copy link
Member

Choose a reason for hiding this comment

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

Can you give an example of how to implement a scorecard_function_?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The scorecard functions are simply a part of the main yaml parser and just change the way we handle a submap. I have the scorecard_function_length in my old implementation.

Parser to get the interfaces we are checking: https://github.com/AlexNPavel/operator-sdk/blob/scorecard-usertest//commands/operator-scorecard/lib/lib.go#L284:L296
Length function: https://github.com/AlexNPavel/operator-sdk/blob/scorecard-usertest//commands/operator-scorecard/lib/lib.go#L198:L213

The only functions I can think of that we would need are array length and regex

Copy link
Member

Choose a reason for hiding this comment

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

Ok it would be good to at least list those two in this section.

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 7, 2019
@estroz
Copy link
Member

estroz commented Nov 9, 2019

@jmccormick2001 is this doc still relevant after the changes you've made? If so we should probs get this merged @AlexNPavel.

@estroz
Copy link
Member

estroz commented Nov 9, 2019

/retest

@jmccormick2001
Copy link
Contributor

@jmccormick2001 is this doc still relevant after the changes you've made? If so we should probs get this merged @AlexNPavel.

I'd probably wait to merge it since we have just entered new stories to define a new 'custom test' design/interface that would work with the other scorecard changes in the queue all as part of v1alpha2 API changes.

Copy link
Member

@estroz estroz 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-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 11, 2019
Copy link
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

Since it is old proposal thas been open since 29 May, I am ok with too.
But the new ones we should be doing following the template which as added in the project.

@estroz
Copy link
Member

estroz commented Nov 12, 2019

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 12, 2019
@camilamacedo86
Copy link
Contributor

Why @estroz /hold this one? What is missing for we merge this?

@camilamacedo86 camilamacedo86 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 19, 2019
@openshift-bot
Copy link

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 19, 2019
@openshift-bot
Copy link

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci-robot
Copy link

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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.

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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. scorecard Issue relates to the scorecard subcomponent size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants