Skip to content

📖 developing e2e tests #2839

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 1 commit into from
May 15, 2020
Merged

📖 developing e2e tests #2839

merged 1 commit into from
May 15, 2020

Conversation

fabriziopandini
Copy link
Member

What this PR does / why we need it:
this PR documents how to write E2E tests that

  • Use the Cluster API test framework.
  • Reflects real user workflow, e.g. Cluster API quick start using clusterctl
  • Can run with different infrastructure providers.

The PR is WIP because some of the described steps is still beinf developed

Which issue(s) this PR fixes:
xref #2753
xref #2637
xref #2636

/assign @vincepri
/assign @sedefsavas
/cc @wfernandes
/cc @gab-satchi

@k8s-ci-robot k8s-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 Apr 1, 2020
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 1, 2020
sedefsavas
sedefsavas reviewed Apr 2, 2020
View reviewed changes
Copy link

@sedefsavas sedefsavas left a comment

Choose a reason for hiding this comment

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

Some nits.

@fabriziopandini
Copy link
Member Author

@sedefsavas thanks for the early feedback!

wfernandes
wfernandes reviewed Apr 3, 2020
View reviewed changes
Copy link
Contributor

@wfernandes wfernandes left a comment

Choose a reason for hiding this comment

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

Just some minor things I noticed.

@fabriziopandini fabriziopandini force-pushed the book-developing-e2e-test branch from 73176c4 to 0e46acc Compare April 27, 2020 10:17
@fabriziopandini fabriziopandini changed the title [WIP] 📖 developing e2e tests 📖 developing e2e tests Apr 27, 2020
@k8s-ci-robot k8s-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 Apr 27, 2020
@fabriziopandini
Copy link
Member Author

This one is ready for review now

sedefsavas
sedefsavas reviewed Apr 28, 2020
View reviewed changes
Copy link

@sedefsavas sedefsavas left a comment

Choose a reason for hiding this comment

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

Thanks @fabriziopandini

Some minor comments.
As a follow-up PR, we may add a section to https://github.com/kubernetes-sigs/cluster-api/blob/0e46acc2515321ee225f5ad6904c674a8913b4c8/docs/book/src/developer/testing.md on how to run the new e2e tests.

docs/book/src/developer/e2e.md

<h1>Deprecated E2E config file format</h1>

The [Cluster API test framework] includes also a [deprecated E2E config file] implementation,
Copy link

Choose a reason for hiding this comment

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

We need to add // Deprecated command for this struct?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes please, it seems I forgot to do this when doing the framework re-shuffle

docs/book/src/developer/e2e.md

<aside class="note">

<h1>Deprecated E2E config file format</h1>
Copy link

Choose a reason for hiding this comment

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

h1 tag looks larger than ### and I think we want to put this deprecated config under ### Defining an E2E config file

Copy link
Member Author

Choose a reason for hiding this comment

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

this is an h1 into an aside tag, not a top level h1 (the preview looks as you are expecting)

docs/book/src/developer/e2e.md

<aside class="note">

<h1>Deprecated InitManagementCluster method</h1>
Copy link

Choose a reason for hiding this comment

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

Same comment as above. Instead of <h1>, #### may be more suitable here.

Copy link
Member Author

Choose a reason for hiding this comment

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

same

@fabriziopandini fabriziopandini force-pushed the book-developing-e2e-test branch from 0e46acc to 8476213 Compare April 28, 2020 09:07
@fabriziopandini
Copy link
Member Author

@sedefsavas thanks for the feedback!

@sedefsavas
Copy link

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 28, 2020
@fabriziopandini
Copy link
Member Author

@vincepri PTAL

@sedefsavas sedefsavas mentioned this pull request May 15, 2020
Merged
@vincepri
Copy link
Member

/milestone v0.3.6
/approve

@k8s-ci-robot k8s-ci-robot added this to the v0.3.6 milestone May 15, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini, vincepri

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [fabriziopandini,vincepri]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 15, 2020
@k8s-ci-robot k8s-ci-robot merged commit beff1e1 into kubernetes-sigs:master May 15, 2020
@fabriziopandini fabriziopandini deleted the book-developing-e2e-test branch May 24, 2020 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gab-satchi gab-satchi Awaiting requested review from gab-satchi

2 more reviewers

@wfernandes wfernandes wfernandes left review comments

@sedefsavas sedefsavas sedefsavas left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@vincepri vincepri

@sedefsavas sedefsavas

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", 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

v0.3.6

Development

Successfully merging this pull request may close these issues.

5 participants

@fabriziopandini @sedefsavas @vincepri @k8s-ci-robot @wfernandes