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

Fixing e2e vs integration test naming #1878

Conversation

amitkrout
Copy link
Contributor

@amitkrout amitkrout commented Jul 1, 2019

What is the purpose of this change? What does it change?

Was the change discussed in an issue?

related to #1780

How to test changes?

Review the change properly

@amitkrout amitkrout force-pushed the e2eVsIntegrationCleanup branch 2 times, most recently from 0cc0058 to 52e7a22 Compare July 4, 2019 09:33
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Jul 5, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Jul 8, 2019
@amitkrout amitkrout changed the title Fixing e2e vs integration test naming [WIP] Fixing e2e vs integration test naming Jul 8, 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. Required by Prow. label Jul 8, 2019
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Jul 9, 2019
@kadel
Copy link
Member

kadel commented Jul 15, 2019

ping @amitkrout

Makefile Outdated
# Run core beta flow e2e tests
.PHONY: test-e2e-beta
test-e2e-beta:
go test -v github.com/openshift/odo/tests/e2escenarios --ginkgo.focus="odo core beta flow" -ginkgo.slowSpecThreshold=$(SLOW_SPEC_THRESHOLD) -ginkgo.v -timeout $(TIMEOUT)
Copy link
Member

@kadel kadel Jul 15, 2019

Choose a reason for hiding this comment

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

why go test here but other commands use ginkgo ?

It would be nice to unify it all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, will fix those.

@@ -12,5 +12,7 @@ make bin
export PATH="$PATH:$(pwd)"
export CUSTOM_HOMEDIR="/tmp/artifacts"

make test-e2e-scenarios
make test-e2e-beta
Copy link
Member

Choose a reason for hiding this comment

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

wouldn't it be better to use make test-e2e-all to make sure that new e2e scenarios are executed once added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1

Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this use test-e2e-all?

@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. labels Jul 29, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Aug 1, 2019
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Aug 6, 2019
@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. labels Aug 6, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Aug 9, 2019
@amitkrout
Copy link
Contributor Author

amitkrout commented Sep 5, 2019

odo sub component command tests when component is in the current directory and --project flag is used 
  creates and pushes local nodejs component and then deletes --all
  /go/src/github.com/openshift/odo/tests/integration/component.go:329
 Created dir: /tmp/724815433
Creating a new project: texjxuxufc
Running odo with args [odo project create texjxuxufc -w -v4]
[odo] I0904 13:18:24.552396   19818 preference.go:118] The path for preference file is /tmp/724815433/config.yaml
[odo] I0904 13:18:24.552472   19818 occlient.go:479] Trying to connect to server api.ci-op-d1whffy3-f09f4.origin-ci-int-aws.dev.rhcloud.com:6443
[odo] I0904 13:18:24.567450   19818 occlient.go:486] Server https://api.ci-op-d1whffy3-f09f4.origin-ci-int-aws.dev.rhcloud.com:6443 is up
[odo] I0904 13:18:24.631580   19818 occlient.go:409] isLoggedIn err:  <nil> 
[odo]  output: "developer"
[odo]  •  Waiting for project to come up  ...
[odo]  ✓  Waiting for project to come up [353ms]
[odo]  ✓  Project 'texjxuxufc' is ready for use
[odo]  ✓  New project created and now using project : texjxuxufc
[odo] I0904 13:18:25.006691   19818 odo.go:70] Could not get the latest release information in time. Never mind, exiting gracefully :)
Current working dir: /go/src/github.com/openshift/odo/tests/integration
Setting current dir to: /tmp/724815433
Running odo with args [odo component create nodejs my-component --app app --project texjxuxufc --env key=value,key1=value1]
[odo]  •  Validating component  ...
[odo]  ✓  Validating component [37ms]
[odo] Please use `odo push` command to create the component with source deployed



[odo] 
Running odo with args [odo component push --context /tmp/724815433]
[odo]  ✗  invalid configuration: [context was not found for specified context: jgjryhzmbp/api-ci-op-d1whffy3-f09f4-origin-ci-int-aws-dev-rhcloud-com:6443/developer, cluster has no server defined]
[odo] Please login to your server: 
[odo] 
[odo] odo login https://mycluster.mydomain.com



[odo] 
Setting current dir to: /go/src/github.com/openshift/odo/tests/integration
Deleting project: texjxuxufc
Running odo with args [odo project delete texjxuxufc -f]
[odo]  •  Deleting project texjxuxufc  ...
[odo]  ✓  Deleting project texjxuxufc [5s]
[odo]  ✓  Deleted project : texjxuxufc
Deleting dir: /tmp/724815433
 • Failure [6.659 seconds]
odo sub component command tests
/go/src/github.com/openshift/odo/tests/integration/cmd_cmp_sub_test.go:13
  when component is in the current directory and --project flag is used
  /go/src/github.com/openshift/odo/tests/integration/component.go:306
    creates and pushes local nodejs component and then deletes --all [It]
    /go/src/github.com/openshift/odo/tests/integration/component.go:329
     No future change is possible.  Bailing out early after 0.102s.
      
    Running odo with args [odo component push --context /tmp/724815433]
    Expected
        <int>: 1
    to match exit code:
        <int>: 0
     /go/src/github.com/openshift/odo/tests/helper/helper_run.go:32

Could be a potential flake
/test integration
Issue created - #2088

@amitkrout
Copy link
Contributor Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Sep 5, 2019
@kadel
Copy link
Member

kadel commented Sep 5, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Sep 5, 2019
@kadel
Copy link
Member

kadel commented Sep 5, 2019

After this gets merged you should open PR against release repo to stop using the old scripts

  • scripts/openshiftci-presubmit-e2e-scenarios.sh
  • scripts/openshiftci-presubmit-e2e.sh

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. Required by Prow. estimated-size/XL (40-60) Rough sizing for Epics. About 3 sprints of work for a person lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants