opentelemetry-demo
opentelemetry-demo copied to clipboard
open-telemetry
🐛 Fixing tests configuration
Fixes https://github.com/open-telemetry/opentelemetry-demo/issues/305.
Changes
There is an issue with some of the test configurations that weren't caught before merging the prev PR.
@mviitane Do you mind taking a look?
@xoscar I can still see the same problem when running with your changes.
Just to be clear, I haven't installed cypress or ava on this machine beforehand, and I think I shouldn't have to, but is that correct?
$ docker compose run frontendTests
[+] Running 13/0
⠿ Container redis-cart Running 0.0s
⠿ Container jaeger Running 0.0s
⠿ Container otel-col Running 0.0s
⠿ Container product-catalog-service Running 0.0s
⠿ Container recommendation-service Running 0.0s
⠿ Container cart-service Running 0.0s
⠿ Container email-service Running 0.0s
⠿ Container shipping-service Running 0.0s
⠿ Container ad-service Running 0.0s
⠿ Container payment-service Running 0.0s
⠿ Container currency-service Running 0.0s
⠿ Container checkout-service Running 0.0s
⠿ Container frontend Running 0.0s
libva error: vaGetDriverNameByIndex() failed with unknown libva error, driver_name = (null)
[228:0817/053441.971654:ERROR:sandbox_linux.cc(377)] InitializeSandbox() called with multiple threads in process gpu-process.
[228:0817/053442.010610:ERROR:gpu_memory_buffer_support_x11.cc(44)] dri3 extension not supported.
Your configFile is invalid: /cypress/cypress.config.ts
It threw an error when required, check the stack trace below:
TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts" for /cypress/cypress.config.ts
at new NodeError (node:internal/errors:371:5)
at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:87:11)
at defaultGetFormat (node:internal/modules/esm/get_format:102:38)
at defaultLoad (node:internal/modules/esm/load:21:14)
at ESMLoader.load (node:internal/modules/esm/loader:359:26)
at ESMLoader.moduleProvider (node:internal/modules/esm/loader:280:58)
at new ModuleJob (node:internal/modules/esm/module_job:66:26)
at ESMLoader.#createModuleJob (node:internal/modules/esm/loader:297:17)
at ESMLoader.getModuleJob (node:internal/modules/esm/loader:261:34)
at processTicksAndRejections (node:internal/process/task_queues:96:5)
at async Promise.all (index 0)
at async ESMLoader.import (node:internal/modules/esm/loader:337:24)
at async importModuleDynamicallyWrapper (node:internal/vm/module:437:15)
at async loadFile (/root/.cache/Cypress/10.3.1/Cypress/resources/app/node_modules/@packages/server/lib/plugins/child/run_require_async_child.js:106:14)
at async EventEmitter.<anonymous> (/root/.cache/Cypress/10.3.1/Cypress/resources/app/node_modules/@packages/server/lib/plugins/child/run_require_async_child.js:116:32)
$
$ docker compose run integrationTests
[+] Running 12/0
⠿ Container redis-cart Running 0.0s
⠿ Container jaeger Running 0.0s
⠿ Container otel-col Running 0.0s
⠿ Container cart-service Running 0.0s
⠿ Container email-service Running 0.0s
⠿ Container currency-service Running 0.0s
⠿ Container shipping-service Running 0.0s
⠿ Container ad-service Running 0.0s
⠿ Container payment-service Running 0.0s
⠿ Container product-catalog-service Running 0.0s
⠿ Container recommendation-service Running 0.0s
⠿ Container checkout-service Running 0.0s
> [email protected] test
> ava
sh: ava: not found
@mviitane qq, did you run the build command for both the frontend and the integration tests from the latest commit? I'm not getting those errors 🤔
@mviitane qq, did you run the build command for both the frontend and the integration tests from the latest commit? I'm not getting those errors 🤔
I ran docker compose build --no-cache for the fix-tests branch. It should also build the integrationTests?
@xoscar After re-building with the latest, I got the integrationTests working! However, the frontendTests still have the same problem.
docker compose build frontend --no-cache
docker compose build integrationTests --no-cache
Hey @mviitane thanks for the feedback, I figured out what the issue was, do you mind rebuilding the frontendTests image and rerunning it again? thanks!
@xoscar Great, I got the tests now running! The frontend tests seem to have 2 tests failing, but I'm not sure if the problem is in the tests or the frontend.
In the original issue, I also proposed to add some simple documentation to test/README.md about how to start the tests. What do you think about that?
@mviitane do you mind taking another look, please rebuild the image and rerun it from the latest commit. I ran it multiple times and got all green
@xoscar I got now all the frontend tests passing few times, but still at least half of the times some tests were failing. So some flakiness at least in my env.
Integration tests are passing all the time.
Hey @austinlparker & @mviitane sorry that it took me a while to come back to this PR. With the latest commit, the frontend E2E tests issues should be fixed and I also added an entry to the main Readme file around the test execution and such. Please take a look at let me know what you think. Thanks!
Hello @xoscar,
I'm using Ubuntu 22.04 and getting 2 failing tests. Both because there is no item in the cart.
Here are the screenshots generated by cypress:
When running the make run-tests, I'm also getting those errors:
libva error: vaGetDriverNameByIndex() failed with unknown libva error, driver_name = (null)
[196:0825/200230.341818:ERROR:sandbox_linux.cc(377)] InitializeSandbox() called with multiple threads in process gpu-process.
[196:0825/200230.348329:ERROR:gpu_memory_buffer_support_x11.cc(44)] dri3 extension not supported.
Missing baseUrl in compilerOptions. tsconfig-paths will be skipped
Maybe those are responsible for the failure.
frontend tests passing, integration tests failing, ubuntu 22.04
Attaching to integrationTests
integrationTests |
integrationTests | > [email protected] test
integrationTests | > ava
integrationTests |
integrationTests |
integrationTests | ✔ shipping: order (102ms)
integrationTests | ✔ shipping: quote (145ms)
integrationTests | ✔ shipping: empty quote (144ms)
integrationTests | ✔ ad: get (222ms)
integrationTests | ✔ currency: supported (216ms)
integrationTests | ✔ currency: convert (216ms)
integrationTests | ✔ payment: invalid credit card (192ms)
integrationTests | ✔ payment: amex credit card not allowed (192ms)
integrationTests | ✔ payment: expired credit card (193ms)
integrationTests | ✔ payment: valid credit card (200ms)
integrationTests | ✖ product: search Rejected promise returned by test
integrationTests | ✔ product: list (232ms)
integrationTests | ✖ product: get
integrationTests | ✔ recommendation: list products (248ms)
integrationTests | ✔ email: confirmation (284ms)
integrationTests | ✔ checkout: place order (311ms)
integrationTests | ✔ cart: all (348ms)
integrationTests | ─
integrationTests |
integrationTests | product: search
integrationTests |
integrationTests | test.js:267
integrationTests |
integrationTests | 266: const res = await productSearch({ query: "hold" });
integrationTests | 267: t.is(res.results.length, 2);
integrationTests | 268: t.is(res.results[0].name, "Candle Holder");
integrationTests |
integrationTests | Rejected promise returned by test. Reason:
integrationTests |
integrationTests | TypeError {
integrationTests | message: 'Cannot read properties of undefined (reading \'length\')',
integrationTests | }
integrationTests |
integrationTests | › test.js:267:20
integrationTests |
integrationTests |
integrationTests |
integrationTests | product: get
integrationTests |
integrationTests | test.js:258
integrationTests |
integrationTests | 257: const res = await productGet({ id: "OLJCESPC7Z" });
integrationTests | 258: t.is(res.name, "Sunglasses");
integrationTests | 259: t.truthy(res.description);
integrationTests |
integrationTests | Difference:
integrationTests |
integrationTests | - 'Natiolal Park Foundation Explorascope'
integrationTests | + 'Sunglasses'
integrationTests |
integrationTests | › test.js:258:5
integrationTests |
integrationTests | ─
integrationTests |
integrationTests | 2 tests failed
integrationTests | npm notice
integrationTests | npm notice New minor version of npm available! 8.15.0 -> 8.18.0
integrationTests | npm notice Changelog: <https://github.com/npm/cli/releases/tag/v8.18.0>
integrationTests | npm notice Run `npm install -g [email protected]` to update!
integrationTests | npm notice
integrationTests exited with code 1
make run-tests giving an error as well -
libva error: vaGetDriverNameByIndex() failed with unknown libva error, driver_name = (null)
[195:0829/154620.876677:ERROR:sandbox_linux.cc(377)] InitializeSandbox() called with multiple threads in process gpu-process.
[195:0829/154620.883062:ERROR:gpu_memory_buffer_support_x11.cc(44)] dri3 extension not supported.
Missing baseUrl in compilerOptions. tsconfig-paths will be skipped
Ok everyone, thank you for your patience. I think I finally fixed the tests. The warning at the beginning shouldn't be a problem, tests should pass either way.
For the integration tests I fixed the ones related to the catalog changes. But there is one outstanding issue regarding the C++ service not returning the right amount of supported currencies.
CC: @austinlparker @mviitane @julianocosta89
frontendTests failing with the following:
Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✖ Checkout.cy.ts 00:10 1 - 1 - - │
├────────────────────────────────────────────────────────────────────────────────────────────────┤
│ ✔ Home.cy.ts 00:01 2 2 - - - │
├────────────────────────────────────────────────────────────────────────────────────────────────┤
│ ✔ ProductDetail.cy.ts 00:05 2 2 - - - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✖ 1 of 3 failed (33%) 00:16 5 4 1 - -
integrationTests are working fine:
> [email protected] test
> ava
✔ email: confirmation
✔ payment: invalid credit card
✔ payment: amex credit card not allowed
✔ payment: expired credit card
✔ payment: valid credit card
✔ product: list
✔ product: search
✔ product: get
✔ recommendation: list products
✔ shipping: order
✔ shipping: quote
✔ shipping: empty quote
✔ currency: supported
✔ currency: convert
✔ checkout: place order
✔ cart: all
✔ ad: get (417ms)
─
17 tests passed
With the latest version, I'm not able to get the tests started. For some reason I get
Error response from daemon: network 280c24c522b9ab9573d736848ecd13aaf79198f424f3d1e385a6a61676a72084 not found
The demo app is running fine though.
@xoscar, you could also merge from main to get #350 included.
hey @xoscar is that expected? https://github.com/open-telemetry/opentelemetry-demo/pull/306#issuecomment-1232501206
docker compose run frontendTests
Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✔ Checkout.cy.ts 00:09 1 1 - - - │
├────────────────────────────────────────────────────────────────────────────────────────────────┤
│ ✔ Home.cy.ts 00:01 2 2 - - - │
├────────────────────────────────────────────────────────────────────────────────────────────────┤
│ ✔ ProductDetail.cy.ts 00:05 2 2 - - - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✔ All specs passed! 00:15 5 5 - - -
docker compose run integrationTests
> [email protected] test
> ava
✔ currency: supported
✔ currency: convert
✔ email: confirmation
✔ payment: invalid credit card
✔ payment: amex credit card not allowed
✔ payment: expired credit card
✔ payment: valid credit card
✔ product: list
✔ product: search
✔ recommendation: list products
✔ shipping: quote
✔ shipping: empty quote
✔ shipping: order
✔ ad: get (102ms)
✔ checkout: place order
✔ cart: all (113ms)
✔ product: get (6.9s)
─
17 tests passed
@xoscar just to be sure, I've ran again the tests, and I got an error with the checkout within the frontendTests.
I looks like the tests are flaky 😢
@austinlparker @cartersocha can we prioritize merging this PR? I think even if the tests might be flaky these changes are required to have them running as part of docker compose. We can fix the flaky test in a follow-up PR.
@austinlparker @cartersocha can we prioritize merging this PR? I think even if the tests might be flaky these changes are required to have them running as part of docker compose. We can fix the flaky test in a follow-up PR.
If we can't fix the flaky tests in this PR, why would another PR help? Why would we want the tests to run as part of compose if they aren't reliable?
@xoscar we decided in the sig call to move forward with only the non flaky tests for now. Post v1 will be updated or re-examined.
Please cut the scope down a bit to the reliable tests
This PR was marked stale due to lack of activity. It will be closed in 7 days.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@cartersocha sorry for the late response, I took one week off 😅 I just updated the PR skipping the flaky tests. Let me know what you think. @austinlparker
The frontend tests seem stable now
Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✔ Checkout.cy.ts 20ms 1 - - 1 - │
├────────────────────────────────────────────────────────────────────────────────────────────────┤
│ ✔ Home.cy.ts 00:03 2 2 - - - │
├────────────────────────────────────────────────────────────────────────────────────────────────┤
│ ✔ ProductDetail.cy.ts 20ms 2 - - 2 - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✔ All specs passed! 00:03 5 2 - 3 -
The integration tests run successfully when first starting the app with
docker compose up
And then using another terminal
docker compose run integrationTests
@julianocosta89, @austinlparker could we get another pass of reviews here?
@julianocosta89 @austinlparker @cartersocha done 😄 I added the quote service as a dependency of the integrationTests