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

Thingpeida integration test using k8s #1041

Merged
merged 4 commits into from
Jul 15, 2021
Merged

Thingpeida integration test using k8s #1041

merged 4 commits into from
Jul 15, 2021

Conversation

jgd5
Copy link
Contributor

@jgd5 jgd5 commented Jul 13, 2021

This PR sets up a k8s cluster on Travis using kind .
Most of the tests are ran inside the frontend pod.

@jgd5 jgd5 force-pushed the wip/integration-test branch 3 times, most recently from a94d066 to e850d5f Compare July 13, 2021 06:46
@jgd5 jgd5 requested a review from gcampax July 13, 2021 07:50
Copy link
Contributor

@gcampax gcampax left a comment

Choose a reason for hiding this comment

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

Great job! This looks very nice, I'm looking forward to automatic testing of our k8s stuff before we deploy to staging. I have left a couple comments but overall I like this a lot.

tests/thingpedia-integration/k8s/config.d/stanford.js Outdated Show resolved Hide resolved
@@ -0,0 +1,21 @@
DATABASE_URL: "mysql://root:password@mariadb.default.svc.cluster.local/thingengine_test?charset=utf8mb4_bin"
DATABASE_PROXY_URL: "http://dbproxy.default.svc.cluster.local:8200"
SERVER_ORIGIN: "http://localhost:8080"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this right? The tests read SERVER_ORIGIN to know where to connect to, so shouldn't it be an in-cluster URL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The localhost:8080 is port mapped to the frontend node port for the selenium tests. Not sure how to get firefox to run inside REHL 8 UBI. The other tests are ran locally inside the frontend pod so localhost:8080 also works.

Copy link
Contributor

Choose a reason for hiding this comment

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

No running firefox from outside makes sense, let Travis deal with configuring it...

tests/thingpedia-integration/Dockerfile Show resolved Hide resolved
THINGENGINE_CONFIGDIR=tests/thingpedia-integration/k8s npx nyc ts-node tests/test_website_selenium.js

# Copy coverage outputs from frontend pod
kubectl cp $POD:/opt/almond-cloud/.nyc_output .nyc_output
Copy link
Contributor

Choose a reason for hiding this comment

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

Is /opt/almond-cloud writable from inside the test docker? Do we get any coverage at all from this process?

And what about Go coverage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is in the frontend pod since I enabled root. I think these coverage outputs are from the thingpedia integration tests.
Go coverage is only enabled in the GO unittests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah coverage of the test code itself is not very interesting. It would be nice to enable coverage of the actual frontend process. Not sure how we do that, and how we kill the frontend and capture it...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got the coverage outputs of frontend and worker. Now the coverage report looks reasonable.

@jgd5 jgd5 force-pushed the wip/integration-test branch 2 times, most recently from a77d026 to 5051459 Compare July 13, 2021 22:35
@coveralls
Copy link

coveralls commented Jul 13, 2021

Pull Request Test Coverage Report for Build 3515

  • 13 of 13 (100.0%) changed or added relevant lines in 1 file are covered.
  • 56 unchanged lines in 6 files lost coverage.
  • Overall coverage increased (+1.4%) to 66.216%

Files with Coverage Reduction New Missed Lines %
src/almond/platform.ts 1 57.67%
src/util/import_device.ts 1 83.96%
src/almond/worker.ts 2 80.34%
src/util/config_init.js 2 67.07%
src/almond/enginemanagerclient.ts 11 72.25%
src/almond/enginemanager.ts 39 75.12%
Totals Coverage Status
Change from base Build 3496: 1.4%
Covered Lines: 7525
Relevant Lines: 10709

💛 - Coveralls

@jgd5 jgd5 force-pushed the wip/integration-test branch 6 times, most recently from cae5496 to 6df9beb Compare July 14, 2021 19:14
@@ -359,7 +359,7 @@ async function testMyApiConverse(auth) {
}, {
id: 4,
type: 'text',
text: 'Sorry, I did not understand that. Can you rephrase it?',
text: 'Sorry, there seems to be a problem with the Genie service at the moment. Please try again later.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Hold on from making this change. The NLP service is having some issue (stanford-oval/genienlp#174) and is not staying up, but this is not the right reply.

@jgd5 jgd5 force-pushed the wip/integration-test branch 2 times, most recently from 6df9beb to 5a5ce18 Compare July 14, 2021 20:52
@jgd5 jgd5 merged commit fffdb64 into master Jul 15, 2021
@jgd5 jgd5 deleted the wip/integration-test branch July 15, 2021 01:57
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.

3 participants