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

feat(httpendpoint): add tests for cli #1280

Merged
merged 26 commits into from
May 30, 2023
Merged

Conversation

sicoyle
Copy link
Contributor

@sicoyle sicoyle commented May 4, 2023

Description

Add checks for new Dapr resource httpendpoints for external service invocation as per comment here.

Issue reference

dapr/dapr#6227

Please reference the issue this PR will close: #[4549]

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

Signed-off-by: Samantha Coyle <sam@diagrid.io>
@sicoyle sicoyle requested review from a team as code owners May 4, 2023 17:20
@sicoyle sicoyle changed the title feat(httpenpoint): add tests for cli feat(httpendpoint): add tests for cli May 4, 2023
artursouza and others added 7 commits May 4, 2023 10:24
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
@sicoyle
Copy link
Contributor Author

sicoyle commented May 5, 2023

Hey @mukundansundar!
Do you have any pointers on e2e tests for v1.11.0 features? I added the external service invocation functionality with the new HTTPEndpoint Dapr resource, and am having a hard time with configuring the e2e tests for it.

Does the runtime v1.11.0 release have to be cut before those would pass? I feel like this is almost a chicken before the egg scenario here... I saw the CRDs work off the raw data for the specific runtime version yaml files so I tried a work around pointing to master for the HTTPEndpoint yaml file, but then it tried to look for a "master" helm-chart release which does not exist. I also see TODO comments in common.go stating TODO: Skip the dashboard version check for now until the helm chart is updated, so that had me thinking that I also need the updated helm chart.

@pravinpushkar
Copy link
Contributor

This will depend on latest runtime changes and will need runtime RC at-least.

@sicoyle
Copy link
Contributor Author

sicoyle commented May 5, 2023

This will depend on latest runtime changes and will need runtime RC at-least.

Thank you @pravinpushkar!! I will circle back here then when the release is ready!

@pravinpushkar
Copy link
Contributor

@sicoyle dapr RC is out. Probably this PR can make use of that.

@mukundansundar
Copy link
Collaborator

@sicoyle dapr RC is out. Probably this PR can make use of that.

I think 1284 needs to be merged before that.

Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
@codecov
Copy link

codecov bot commented May 26, 2023

Codecov Report

Merging #1280 (d44d942) into master (36a13bc) will not change coverage.
The diff coverage is n/a.

❗ Current head d44d942 differs from pull request most recent head e5e5522. Consider uploading reports for the commit e5e5522 to get more accurate results

@@           Coverage Diff           @@
##           master    #1280   +/-   ##
=======================================
  Coverage   27.03%   27.03%           
=======================================
  Files          39       39           
  Lines        3873     3873           
=======================================
  Hits         1047     1047           
  Misses       2752     2752           
  Partials       74       74           

Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
@sicoyle
Copy link
Contributor Author

sicoyle commented May 26, 2023

E2E self hosted failure seems unrelated and due to timing out on installing podman:

Waiting for VM ...
Error: The action has timed out.

also upload test results failure shows:

Run actions/upload-artifact@master
Error: Input required and not supplied: path

@sicoyle
Copy link
Contributor Author

sicoyle commented May 27, 2023

I think there may be a github issue or workflow connectivity issue going on for that last build because looking through the E2E upgrade path tests I see it's failing because of timing issues again:

For example, common.go:792 is from a 2 minute timeout that expired waiting for pods to delete.

goroutine 786 [chan send, 18 minutes]:
github.com/dapr/cli/tests/e2e/common.waitPodDeletion(0xc0004ccfd0?, 0xf51f2a?, 0x0?)
	/home/runner/work/cli/cli/src/github.com/dapr/cli/tests/e2e/common/common.go:1031 +0x1f1
created by github.com/dapr/cli/tests/e2e/common.uninstallTest.func1
	/home/runner/work/cli/cli/src/github.com/dapr/cli/tests/e2e/common/common.go:792 +0x198

Signed-off-by: Samantha Coyle <sam@diagrid.io>
@sicoyle
Copy link
Contributor Author

sicoyle commented May 27, 2023

documenting again bc I've seen this issue inconsistently as well in my many e2e test failures 😅 The inconsistency part is definitely a timing related issue we should capture and address in the future; however, for the time being, this should have a conditional since the dapr dashboard is removed from the helm chart in v1.11 RC here. I am opening a PR to address separately.

=== RUN   TestUpgradePathNonHAModeMTLSDisabled/vv1.11.0-rc.4_to_v1.10.5/clusterroles_exist_v1.11.0-rc.4
    common.go:454: 
        	Error Trace:	/home/runner/work/cli/cli/src/github.com/dapr/cli/tests/e2e/common/common.go:454
        	Error:      	Not equal: 
        	            	expected: true
        	            	actual  : false
        	Test:       	TestUpgradePathNonHAModeMTLSDisabled/vv1.11.0-rc.4_to_v1.10.5/clusterroles_exist_v1.11.0-rc.4
        	Messages:   	cluster role dapr-dashboard, found = false, wanted = true

yaron2 and others added 2 commits May 29, 2023 18:08
tests/e2e/upgrade/upgrade_test.go Outdated Show resolved Hide resolved
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
@yaron2 yaron2 merged commit 96e2254 into dapr:master May 30, 2023
shivam-51 pushed a commit to shivam-51/cli that referenced this pull request Jun 1, 2023
* feat(httpenpoint): add tests for cli

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* feat(httpendpoints): add httpendpoints in another place in tests

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix(e2e): adjust checks for httpendpoints

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* tests(e2e): more adjustments for httpendpoints

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix(e2e): keep in mind dapr versions for tests

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix(e2e): try workaround until 1.11 release

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix(e2e): only test httpendpoints on new tests

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix(tests): check correct resource output

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* style: make lint

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix(tests): only check httpendpoints if opt enabled

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix(tests): only check httpendpoints if opt enabled for uninstall

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix(e2e): use latest rc

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* style: make lint again

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* test: try this

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix(test): correct expected output string

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix(tests): master -> rc version

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix(tests): acct for ns already being deleted

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* style: make linter happy

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix(tests): try a modification

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* build: retrigger build with random comment update

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix(tests): rm extra test case

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix: address pr feedback

Signed-off-by: Samantha Coyle <sam@diagrid.io>

* fix: update based on pr feedback again

Signed-off-by: Samantha Coyle <sam@diagrid.io>

---------

Signed-off-by: Samantha Coyle <sam@diagrid.io>
Co-authored-by: Artur Souza <artursouza.ms@outlook.com>
Co-authored-by: Yaron Schneider <schneider.yaron@live.com>
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.

Proposal: Extend service invocation to non-Dapr endpoints
5 participants