Skip to content

Add test/check sanity to check if test-framework mock data is updated #2131

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Nov 14, 2019
Merged

Add test/check sanity to check if test-framework mock data is updated #2131

merged 4 commits into from
Nov 14, 2019

Conversation

camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Oct 30, 2019 •
edited
Loading

Description of the change:

  • Add test/check sanity to check if test-framework mock data is updated

PS.: The only change here is the commit 499e108. The other files changed in this PR are cherry-picks from #2130 and #2129. We need to merge them first.

Motivation for the change:

Avoid the tests been executed with code implementations which are no longer valid for the product and in this way, avoid issues.

Directly mock data test-framework usage

  • In unit tests as for example crd_test.go scaffold checks. See here and here
  • In go e2e tests. See: here and here
  • in the subcommands checks. See here
  • The CI image dockerfile will use its bin. See here
  • In the scorecard tests. See here and here
  • Also, users are addressed to check its implementation as an example since it is a really valid mock data in the project. See, here and here

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 30, 2019
@camilamacedo86 camilamacedo86 changed the title Test sanity framework data Test sanity checking if the mock framework data is matching with the current source code implementation Oct 30, 2019
@camilamacedo86 camilamacedo86 changed the title Test sanity checking if the mock framework data is matching with the current source code implementation WIP: Test sanity checking if the mock framework data is matching with the current source code implementation Oct 31, 2019
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 31, 2019
@camilamacedo86 camilamacedo86 changed the title WIP: Test sanity checking if the mock framework data is matching with the current source code implementation Add test/check sanity to ensure that the test-framework data will be always updated. Nov 1, 2019
@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 Nov 1, 2019
@camilamacedo86
Copy link
Contributor Author

/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 1, 2019
@camilamacedo86 camilamacedo86 requested a review from estroz November 1, 2019 15:37
hasbro17
hasbro17 reviewed Nov 5, 2019
View reviewed changes
@camilamacedo86 camilamacedo86 changed the title Add test/check sanity to ensure that the test-framework data will be always updated. Add test/check sanity to check if test-framework mock data is updated Nov 6, 2019
@camilamacedo86 camilamacedo86 mentioned this pull request Nov 7, 2019
Merged
@camilamacedo86 camilamacedo86 added test-framework kind/feature Categorizes issue or PR as related to a new feature. labels Nov 7, 2019
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 9, 2019
@camilamacedo86 camilamacedo86 removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 9, 2019
joelanford
joelanford requested changes Nov 11, 2019
View reviewed changes
@camilamacedo86 camilamacedo86 dismissed stale reviews from joelanford and asmacdo via 5aeaae6 November 13, 2019 16:35
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 13, 2019
@camilamacedo86
Copy link
Contributor Author

/test marker

@camilamacedo86
Copy link
Contributor Author

/retest

@camilamacedo86
Copy link
Contributor Author

/test marker

joelanford
joelanford previously approved these changes Nov 13, 2019
View reviewed changes
Copy link
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

LGTM

estroz
estroz suggested changes Nov 13, 2019
View reviewed 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.

A few nits.

@estroz
Copy link
Member

estroz commented Nov 13, 2019

@camilamacedo86 I'll make a follow-up PR that adds a Dockerfile and go.mod/go.sum to test/test-framework so we can treat it like a full project/submodule.

camilamacedo86
camilamacedo86 commented Nov 13, 2019
View reviewed changes
@camilamacedo86
add sanity check to ensure that the test-framework is updated
8ab602c
@camilamacedo86
changes requested in the review
bf3257a 
adding suggestions on rview

fix nits review

remove spaces at the end of the file

using go mod init
@camilamacedo86
suggestion
8adf59b
@camilamacedo86
last nits
69aa1ed
estroz
estroz approved these changes Nov 14, 2019
View reviewed 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 14, 2019
joelanford
joelanford reviewed Nov 14, 2019
View reviewed changes
Copy link
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

LGTM

@camilamacedo86 camilamacedo86 merged commit 79e6369 into operator-framework:master Nov 14, 2019
@camilamacedo86 camilamacedo86 deleted the test-sanity-framework-data branch November 14, 2019 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joelanford joelanford joelanford approved these changes

@estroz estroz estroz approved these changes

@AlexNPavel AlexNPavel Awaiting requested review from AlexNPavel

@jmrodri jmrodri Awaiting requested review from jmrodri

@jmccormick2001 jmccormick2001 Awaiting requested review from jmccormick2001

@hasbro17 hasbro17 Awaiting requested review from hasbro17

@theishshah theishshah Awaiting requested review from theishshah

@asmacdo asmacdo Awaiting requested review from asmacdo

Assignees

@estroz estroz

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@camilamacedo86 @openshift-bot @estroz @joelanford @asmacdo @hasbro17 @openshift-ci-robot