search

dapr / cli

Command-line tools for Dapr.
Apache License 2.0
315 stars 200 forks source link

feat(httpendpoint): add tests for cli #1280

Closed sicoyle closed 1 year ago

sicoyle commented 1 year ago

Description

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

Issue reference

https://github.com/dapr/dapr/pull/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:

sicoyle commented 1 year ago

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 commented 1 year ago

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

sicoyle commented 1 year ago

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 commented 1 year ago

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

mukundansundar commented 1 year ago

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

I think 1284 needs to be merged before that.

codecov[bot] commented 1 year ago

Codecov Report

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

:exclamation: 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
sicoyle commented 1 year ago

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 commented 1 year ago

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
sicoyle commented 1 year ago

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