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

[WIP] Update testing steps in CI #200

Closed

Conversation

shubham1172
Copy link
Member

@shubham1172 shubham1172 commented Feb 23, 2022

Description

Sorry for slightly multi-functional PR. The changes are closely related to each other. It does the following:

  1. Separates unit tests from e2e tests - unit tests are now a part of the build pipeline.
  2. Fixes e2e testing issue (Pipeline passes even though e2e test fails #197)
  3. Introduces codecov (Code coverage reporting #193)
  4. Adds utils unit tests to package.json

Issue reference

Please reference the issue this PR will close #193 and close #197

Checklist

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

  • (NA) Code compiles correctly
  • (NA) Created/updated tests
  • (NA) Extended the documentation

Signed-off-by: Shubham Sharma <shubhash@microsoft.com>
Signed-off-by: Shubham Sharma <shubhash@microsoft.com>
@shubham1172 shubham1172 requested review from a team as code owners February 23, 2022 11:16
@codecov
Copy link

codecov bot commented Feb 23, 2022

Codecov Report

❗ No coverage uploaded for pull request base (master@79ae288). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 6cfd484 differs from pull request most recent head 5e59aa8. Consider uploading reports for the commit 5e59aa8 to get more accurate results

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #200   +/-   ##
=========================================
  Coverage          ?   26.98%           
=========================================
  Files             ?       70           
  Lines             ?     6141           
  Branches          ?      976           
=========================================
  Hits              ?     1657           
  Misses            ?     4102           
  Partials          ?      382           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79ae288...5e59aa8. Read the comment docs.

Signed-off-by: Shubham Sharma <shubhash@microsoft.com>
Signed-off-by: Shubham Sharma <shubhash@microsoft.com>
Signed-off-by: Shubham Sharma <shubhash@microsoft.com>
Signed-off-by: Shubham Sharma <shubhash@microsoft.com>
Signed-off-by: Shubham Sharma <shubhash@microsoft.com>
Signed-off-by: Shubham Sharma <shubhash@microsoft.com>
Signed-off-by: Shubham Sharma <shubhash@microsoft.com>
Signed-off-by: Shubham Sharma <shubhash@microsoft.com>
@XavierGeerinck
Copy link
Contributor

@shubham1172 could you walk me through the idea you are executing here?

Also: I see the replacement with TestIt, but I think the util is commented out?

@shubham1172
Copy link
Member Author

@XavierGeerinck, it's currently a WIP, I was trying out different things.

The first approach is something similar to what conformance tests in https://github.com/dapr/components-contrib do. We can set an environment variable on failure, and use that to detect in the next step. This is a hack, but should be a good short term solution till we reach the second approach. Also, the code is commented because the CI started going crazy in an infinite loop, didn't want it to break, LOL.

The second approach will be like what Java SDK/.NET SDK do today. Reference: https://github.com/dapr/java-sdk/tree/master/sdk-tests/src/test/java/io/dapr/it. They have an elaborate test runner, that is responsible for starting up Dapr
on random ports, with timeouts and retries in place. It listens for DAPR logs, and looks for "success criteria". The test runner invokes dapr run with command to run the tests (similar to our test:e2e:all in package.json), but since the control is with the test runner now, we are free to check the logs and decide if the tests passed.

The second approach is better than first, but will take more time. I think once this is fixed, dapr/cli#684, our original method should work, but that change might require a lot of time since people would already have dependencies in place.

@shubham1172
Copy link
Member Author

As discusses offline, we are leaning to provide a short-term fix using first approach, and then plan the second approach as a long-term fix.

Signed-off-by: Shubham Sharma <shubhash@microsoft.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.

Pipeline passes even though e2e test fails Code coverage reporting
2 participants