Tests cleanup #53

Merged
ashwin merged 11 commits from deep-stack/laconic-sdk:pm-tests-cleanup into main 2024-01-22 08:30:36 +00:00
Member

General test improvements.

  • Simplifies Dockerfile
  • Refactors tests so they can be run independently
General test improvements. - Simplifies Dockerfile - Refactors tests so they can be run independently
prathamesh added 3 commits 2024-01-15 08:41:36 +00:00
dockerfile cleanup
All checks were successful
Tests / sdk_tests (pull_request) Successful in 19m25s
913389ab9f
prathamesh changed title from Tests cleanup to [WIP] Tests cleanup 2024-01-15 08:51:07 +00:00
prathamesh added 1 commit 2024-01-16 09:04:55 +00:00
Add container build script and update workflow
Some checks failed
Tests / sdk_tests (pull_request) Failing after 5m32s
e5900b2e4a
prathamesh added 1 commit 2024-01-16 09:57:57 +00:00
Update container build script
Some checks failed
Tests / sdk_tests (pull_request) Failing after 4m55s
b7223c1c43
prathamesh added 1 commit 2024-01-16 10:49:59 +00:00
Update naming test timeout
Some checks failed
Tests / sdk_tests (pull_request) Failing after 4m54s
603fe5916a
prathamesh added 1 commit 2024-01-16 12:05:30 +00:00
Update naming test timeout
Some checks failed
Tests / sdk_tests (pull_request) Failing after 5m10s
972b878674
prathamesh changed title from [WIP] Tests cleanup to Tests cleanup 2024-01-16 12:17:22 +00:00
ashwin requested review from telackey 2024-01-16 12:22:38 +00:00
telackey requested changes 2024-01-16 18:22:12 +00:00
telackey left a comment
Member

I like the look of the changes overall, but the CI job is failing in Gitea. I think we need to get that sorted out before merging.

I like the look of the changes overall, but the CI job is failing in Gitea. I think we need to get that sorted out before merging.
Member

I suspect the failure is related to the new script not using the correct DOCKER_HOST.

This may be corrected with our new task executor containers (I'll try relaunching once deployed), otherwise you'll need to do something like:

run: DOCKER_HOST=unix:///var/run/dind.sock  ./scripts/build-sdk-test-container.sh
I suspect the failure is related to the new script not using the correct DOCKER_HOST. This may be corrected with our new task executor containers (I'll try relaunching once deployed), otherwise you'll need to do something like: ``` run: DOCKER_HOST=unix:///var/run/dind.sock ./scripts/build-sdk-test-container.sh ```
prathamesh added 1 commit 2024-01-17 13:05:17 +00:00
Add debug logs
Some checks failed
Tests / sdk_tests (pull_request) Failing after 11s
0a99a1aea1
prathamesh added 1 commit 2024-01-17 13:10:10 +00:00
Update repo URL
Some checks failed
Tests / sdk_tests (pull_request) Failing after 10m15s
9535362887
prathamesh added 1 commit 2024-01-17 13:26:02 +00:00
Log docker versions in CI
Some checks failed
Tests / sdk_tests (pull_request) Failing after 12s
7db69c314b
prathamesh force-pushed pm-tests-cleanup from 7db69c314b to d7ca4730a2 2024-01-17 13:26:49 +00:00 Compare
Author
Member

but the CI job is failing in Gitea

The reason for failure earlier was that laconicd PR (cerc-io/laconicd#134) is yet to be merged and the CI in this PR has been updated to use changes from there.

However, even if I temporarily point it to use the PR branch, the sdk tests are still failing (naming tests). Have added some logs to investigate this, but needs more debugging.

> but the CI job is failing in Gitea The reason for failure earlier was that laconicd PR (https://git.vdb.to/cerc-io/laconicd/pulls/134) is yet to be merged and the CI in this PR has been updated to use changes from there. However, even if I temporarily point it to use the PR branch, the sdk tests are still failing (naming tests). Have added some logs to investigate this, but needs more debugging.
Author
Member

However, even if I temporarily point it to use the PR branch, the sdk tests are failing (naming tests). Have added some logs to investigate this, but needs more debugging.

The tests do pass locally though; both when laconicd is run on the host machine as well as in docker (as done in the workflow).

> However, even if I temporarily point it to use the PR branch, the sdk tests are failing (naming tests). Have added some logs to investigate this, but needs more debugging. The tests do pass locally though; both when laconicd is run on the host machine as well as in docker (as done in the workflow).
Member

We actually redeployed the task executors with ones that automatically set DOCKER_HOST, so that probably corrected part of it.

As to the naming failures, if it works locally, is the idea that it is some sort of timing/performance issue in CI?

I am just a bit concerned in that, that unlike the laconid tests, which we need to get passing in Gitea, these tests actually were passing, although looking at the history they seem to have been pretty flakey.

I didn't worry about them failing in CI on the last PR, because it seemed to be that the failures were just from the laconicd/protobuf version mismatch and would sort themselves out, but since the PR is about cleaning up the tests, I think it is worth some investigation as to why they are failing.

We actually redeployed the task executors with ones that automatically set DOCKER_HOST, so that probably corrected part of it. As to the naming failures, if it works locally, is the idea that it is some sort of timing/performance issue in CI? I am just a bit concerned in that, that unlike the laconid tests, which we need to get passing in Gitea, these tests actually were passing, although looking at the history they seem to have been pretty flakey. I didn't worry about them failing in CI on the last PR, because it seemed to be that the failures were just from the laconicd/protobuf version mismatch and would sort themselves out, but since the PR is about cleaning up the tests, I think it is worth some investigation as to why they are failing.
Member

One option (and the same goes for cerc-io/laconicd#134) is that if everything is passing for you locally, to go ahead and merge, but open an issue with all the details of the failures, and we will fix the CI as a separate task/PR.

Sound reasonable?

One option (and the same goes for https://git.vdb.to/cerc-io/laconicd/pulls/134) is that if everything is passing for you locally, to go ahead and merge, but open an issue with all the details of the failures, and we will fix the CI as a separate task/PR. Sound reasonable?
telackey requested review from telackey 2024-01-18 18:58:02 +00:00
telackey approved these changes 2024-01-18 18:58:23 +00:00
prathamesh added 1 commit 2024-01-22 06:56:08 +00:00
Remove debug logs
Some checks failed
Tests / sdk_tests (pull_request) Failing after 3m44s
f525e18ca4
Author
Member

One option (and the same goes for cerc-io/laconicd#134) is that if everything is passing for you locally, to go ahead and merge, but open an issue with all the details of the failures, and we will fix the CI as a separate task/PR.

Created an issue to track this: #54

> One option (and the same goes for cerc-io/laconicd#134) is that if everything is passing for you locally, to go ahead and merge, but open an issue with all the details of the failures, and we will fix the CI as a separate task/PR. Created an issue to track this: https://git.vdb.to/cerc-io/laconic-sdk/issues/54
ashwin merged commit fedf35d702 into main 2024-01-22 08:30:36 +00:00
ashwin deleted branch pm-tests-cleanup 2024-01-22 08:30:36 +00:00
ashwin referenced this issue from a commit 2024-01-22 08:30:38 +00:00
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cerc-io/laconic-sdk#53
No description provided.