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 CI testing overview #98

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

dymurray
Copy link

@dymurray dymurray commented Feb 14, 2023

This enhancement document covers a high-level overview of how we plan to begin implementing CI testing starting with an API test suite written in Go.

WIP: Implementation details will be updated further as a PoC is developed.

Signed-off-by: Dylan Murray <[email protected]>
Copy link
Member

@aufi aufi left a comment

Choose a reason for hiding this comment

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

Nice enhancement and starting point for the upstream CI! Added few notes to Proposal and Implementation details sections, but overall, looks good to me.

enhancements/testing/high-level-summary.md Show resolved Hide resolved
I propose we make this a phased approach. The first phase will be building upon
te existing work done by Shveta and Mayaan (see Alternatives section) to
establish a test suite which provides full API test coverage. I propose we
write this test suite in Golang so that all API contributors can provide tests
Copy link
Member

Choose a reason for hiding this comment

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

(nit) Not sure about specifying Golang as a requirement here, but definitely agree on some test technology which would fit well to Tackle Hub API (and other Tackle components).

Copy link
Author

Choose a reason for hiding this comment

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

Maybe not a goal, but I do hope for this enhancement to allow the community to settle on a choice for a language in which these tests will be written since we already have Python tests in the wild.

To begin phase 1, I will work to replicate the functionality of the Python test
suite in a Golang version using a [table-driven
testing](https://dave.cheney.net/2019/05/07/prefer-table-driven-tests)
approach.
Copy link
Member

Choose a reason for hiding this comment

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

I agree on table-driven testing from the blog post in meaning of writing set of tests as a set of requirements what is input and its expected ouput (or performed action result) declared in as easy way as possible and making the underhood logic DRY and reusable for other developers&QEs.

It might be worth put main points of the table-driven testing to be on the same page on its meaning.

Copy link

Choose a reason for hiding this comment

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

+1 for table driven testing . This way we can easily add or remove analysis targets and test various applications with same tests .

tests. The best way to do this would be to keep the test suite in Go. The
framework to be used inside of the test suite doesn't need to be determined
ahead of time, but a framework such as Ginkgo might be worthwhile to explore
for integration testing.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not against this, just want to mention that the test suite that upstream CI should perform is more likely to be end-to-end (or at least integration) testing, that doesn't interact directly with golang code, but only with API (mostly HTTP-based).

That might lead to slightly wider set of possible tools/frameworks (focused on API/integration testing).

The main point here I would highlight is that that the testing tool/framework should fit well to the environment we develop the Tackle and to provide smooth ramp-up for new contributors.

## Proposal

I propose we make this a phased approach. The first phase will be building upon
te existing work done by Shveta and Mayaan (see Alternatives section) to
Copy link
Member

Choose a reason for hiding this comment

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

nit: s/te/the

@@ -0,0 +1,146 @@
---
title: neat-enhancement-idea
Copy link
Member

Choose a reason for hiding this comment

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

The title might need to be updated.

Signed-off-by: Dylan Murray <[email protected]>
Copy link
Member

@djzager djzager left a comment

Choose a reason for hiding this comment

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

I think some great comments were made in konveyor/tackle2-hub#241 that should be incorporated into this enhancement.

Comment on lines 5 to 11
reviewers:
- TBD
approvers:
- TBD
creation-date: 2023-02-08
last-updated: 2023-02-09
status: provisional|implementable|implemented|deferred|rejected|withdrawn|replaced
Copy link
Member

Choose a reason for hiding this comment

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

need reviewers/approvers and an actual status.

Comment on lines 26 to 27
- [ ] Test plan is defined
- [ ] User-facing documentation is created
Copy link
Member

Choose a reason for hiding this comment

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

I feel like, for this enhancement you can drop these.


## Open Questions [optional]

> 1. What tools do we have to do upstream UI testing?
Copy link
Member

Choose a reason for hiding this comment

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

Looks like the tackle ui tests use https://www.cypress.io/


# Konveyor Testing Initiative

It is desired to add upstream CI testing to provide regression and integration
Copy link
Member

Choose a reason for hiding this comment

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

Maybe, instead of "upstream CI testing" we could be referring to an upstream CI strategy...covering a little more than just a single test suite.

testing](https://dave.cheney.net/2019/05/07/prefer-table-driven-tests)
approach.

### User Stories [optional]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### User Stories [optional]
### User Stories


## Summary

This enhancement defines the steps towards establishing a robust CI test suite
Copy link
Member

Choose a reason for hiding this comment

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

After reading what @jortel wrote in konveyor/tackle2-hub#241, I wonder if we should be distinguishing and planning to tackle both the API testing we are talking about here as well as the application level testing mentioned in the "Hub testing strategy".

I feel like we want to be able to test the hub quickly in local environments and in running clusters. We also want to test at the application level -- through the UI.

More research may need to be done but these two buckets seem to represent the highest priority items for us as far as CI strategy goes.

Copy link
Author

Choose a reason for hiding this comment

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

Agreed. I will rework with these as the high level goals.

@dymurray
Copy link
Author

dymurray commented Mar 9, 2023

In the interest of keeping this high level, I will simply cover the overall goals to get our CI infrastructure up in place. Any details on the choices for and API/UI test suite will be in a follow up enhancement.

Signed-off-by: Dylan Murray <[email protected]>
@jwmatthews
Copy link
Member

@aufi @dymurray @djzager @fabianvf thoughts on this for review/ack/merge?

@djzager
Copy link
Member

djzager commented Oct 30, 2023

I suspect this should be cleaned up a bit to better reflect what we've actually done, then merged. It doesn't seem that far off though.

@shawn-hurley
Copy link
Contributor

Considering this mostly exists, can we merge? @dymurray @djzager @aufi

@djzager
Copy link
Member

djzager commented May 2, 2024

Considering this mostly exists, can we merge? @dymurray @djzager @aufi

Seems reasonable to me. Looks like it's blocked on handling the existing comments. Think @dymurray can resolve those as he sees fit.

edit: adding context

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.

6 participants