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

Fix integration and e2e test organization structure #5840

Closed
3 tasks
kadel opened this issue Jun 20, 2022 · 8 comments · Fixed by #5931
Closed
3 tasks

Fix integration and e2e test organization structure #5840

kadel opened this issue Jun 20, 2022 · 8 comments · Fixed by #5931
Assignees
Labels
area/testing Issues or PRs related to testing, Quality Assurance or Quality Engineering kind/user-story An issue of user-story kind priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)).

Comments

@kadel
Copy link
Member

kadel commented Jun 20, 2022

I've just noticed that we still have integration tests separated from devfile tests (#5839).
This organization doesn't make sense anymore.

User Story

  • As an odo developer
  • I want to have a straightforward test structure, and when I'm executing integration tests I want to be sure that all tests are executed
  • So that don't introduce new bugs into existing code that is already covered by tests

/kind user-story

Acceptance criteria

  • To make executing tests more straightforward forward there should be only 3 ways to start tests.
    # unit test
    make test
    
    # integration tests
    make test-integrationn
    
    # e2e tess
    make test-e2e
    
  • no other make test-.. target should exist
  • There should be no tests/integration/devile/ directory, all integration tests should be in tests/integration. Or in another words go run github.com/onsi/ginkgo/ginkgo tests/integration/` should execute all integration tests.

/area user-story
/priority hight

@kadel kadel added priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)). area/testing Issues or PRs related to testing, Quality Assurance or Quality Engineering area/application and removed area/application labels Jun 20, 2022
@openshift-ci
Copy link

openshift-ci bot commented Jun 20, 2022

@kadel: The label(s) area/user-story, priority/hight cannot be applied, because the repository doesn't have them.

In response to this:

I've just noticed that we still have integration tests separated from devfile tests (#5839).
This organization doesn't make sense anymore.

User Story

  • As an odo developer
  • I want to have a straightforward test structure, and when I'm executing integration tests I want to be sure that all tests are executed
  • So that don't introduce new bugs into existing code that is already covered by tests

/kind user-story

Acceptance criteria

  • To make executing tests more straightforward forward there should be only 3 ways to start tests.
# unit test
make test

# integration tests
make test-integrationn

# e2e tess
make test-e2e
  • no other make test-.. target should exist
  • There should be no tests/integration/devile/ directory, all integration tests should be in tests/integration. Or in another words go run github.com/onsi/ginkgo/ginkgo tests/integration/` should execute all integration tests.

/area user-story
/priority hight

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.

@openshift-ci openshift-ci bot added the kind/user-story An issue of user-story kind label Jun 20, 2022
@rm3l
Copy link
Member

rm3l commented Jun 27, 2022

As discussed, Ginkgo v2 should make things much easier for this. So adding a dependency to #5791

/status blocked

@rm3l
Copy link
Member

rm3l commented Jun 27, 2022

Also, there currently is a separate test-interactive target because interactive tests do not run on Windows (cf. #5573). But we could unify all those targets by skipping interactive tests in the test code (as we already do with an environment variable for Kubernetes vs Openshift tests).

@rm3l rm3l added the status/blocked Denotes an issue or PR that is blocked on something (e.g., issue/PR in different repo) label Jun 27, 2022
@rm3l rm3l changed the title Fix integration and e2e test organization strcuture Fix integration and e2e test organization structure Jul 1, 2022
@anandrkskd
Copy link
Contributor

anandrkskd commented Jul 6, 2022

Do we want to remove the tree structure of directories under ./test/integration/ and have only one directory /test/integration that contains all the go test files?
Or do we just want to have a simplified make targets for tests execution?

WDYT? @rm3l @feloy @dharmit

@rm3l rm3l removed the status/blocked Denotes an issue or PR that is blocked on something (e.g., issue/PR in different repo) label Jul 7, 2022
@rm3l
Copy link
Member

rm3l commented Jul 7, 2022

Do we want to remove the tree structure of directories under ./test/integration/ and have only one directory /test/integration that contains all the go test files? Or do we just want to have a simplified make targets for tests execution?

WDYT? @rm3l @feloy @dharmit

I think @kadel's point - and I agree with him - was to have both :-)

  • all integration test files should live in the same tests/integration folder, and all e2e test files stay (as they currently are) in the tests/e2escenarios folder. If we need to skip some tests, we can do it in the test code, rather than creating different Make targets.
  • there is way too many (32) test* targets in the current Makefile. I am not even sure all of these targets are used; we should simplify this by having just 3 targets for running the tests: test, test-integration, and test-e2e.
❯ grep "^test" Makefile | awk -F ':' '{print $1}' | sort
test
test-cmd-debug
test-cmd-delete
test-cmd-devfile-app
test-cmd-devfile-config
test-cmd-devfile-debug
test-cmd-devfile-describe
test-cmd-devfile-env
test-cmd-devfile-exec
test-cmd-devfile-init
test-cmd-devfile-list
test-cmd-devfile-log
test-cmd-devfile-push
test-cmd-devfile-registry
test-cmd-devfile-status
test-cmd-devfile-storage
test-cmd-devfile-test
test-cmd-devfile-watch
test-cmd-link-unlink-4-cluster
test-cmd-login-logout
test-cmd-pref-config
test-cmd-project
test-cmd-watch
test-coverage
test-e2e-all
test-e2e-devfile
test-generic
test-integration
test-integration-devfile
test-interactive
test-plugin-handler
test-windows

@dharmit
Copy link
Member

dharmit commented Jul 7, 2022

I personally struggle with understanding the purpose of e2e tests in the context of odo, and don't remember adding any tests under that directory during the time I have worked on odo.

I think (and would love to be corrected) we are testing end to end scenarios in integration tests. So, I'm not sure why we should have both tests/integration and tests/e2escenarios in odo.

@dharmit
Copy link
Member

dharmit commented Jul 7, 2022

WDYT? @rm3l @feloy @dharmit

Curious as to what @valaparthvi opines. She's been a QE in her past life.

@valaparthvi
Copy link
Contributor

WDYT? @rm3l @feloy @dharmit

Curious as to what @valaparthvi opines. She's been a QE in her past life.

Past life :P

I didn't have much experience using end2end test cases while working on my previous projects, but with odo, I agree with Dharmit that it's slightly difficult to draw a line between our integration tests and e2e tests.

@rm3l rm3l added the v3 label Oct 7, 2022
@rm3l rm3l mentioned this issue Oct 21, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Issues or PRs related to testing, Quality Assurance or Quality Engineering kind/user-story An issue of user-story kind priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)).
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants