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

Add tests to ci #90

Merged
merged 1 commit into from
Aug 13, 2022
Merged

Add tests to ci #90

merged 1 commit into from
Aug 13, 2022

Conversation

KonradStaniec
Copy link
Collaborator

@KonradStaniec KonradStaniec commented Aug 11, 2022

Adds integration tests to CI.

Main change needed to achieve that was to change execution env from docker to linuxVM as docker execution environment does not support communication between primary container and containers started via docket compose

I needed to disable one makefile compilation target i.e # include contrib/devtools/Makefile, due to some weird error which touches make compilation rules:
contrib/devtools/Makefile:61: *** target pattern contains no '%'. Stop.
From what I read this has to something to do most probably with some stray colon -
https://stackoverflow.com/questions/47078108/how-do-i-fix-makefile1-target-pattern-contains-no-stop
https://stackoverflow.com/questions/15113138/target-pattern-contains-no-makefile
although I am unable to pinpoint which, and would prefer to fix it in separate pr to not block this one, as having even simple integration tests will enable catching many potential issues quickly. (like not updated docker containers version etc.)

@KonradStaniec KonradStaniec force-pushed the add-step-to-ci branch 8 times, most recently from b4a893d to 731b690 Compare August 11, 2022 17:35
@KonradStaniec KonradStaniec marked this pull request as ready for review August 11, 2022 17:48
Copy link
Contributor

@aakoshh aakoshh left a comment

Choose a reason for hiding this comment

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

There was some dind package we used in the past to be able to run Docker in Docker:

https://discuss.circleci.com/t/how-to-do-dind-docker-in-docker/32444

@@ -99,7 +99,8 @@ endif
all: tools build lint test

# The below include contains the tools and runsim targets.
include contrib/devtools/Makefile
# TODO: Fix following make file so it will work on linux
Copy link
Contributor

Choose a reason for hiding this comment

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

Strage, it works for me locally.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep it also works on my mac. I intend to install linux vm locally and check it out

@@ -19,19 +20,25 @@ jobs:
command: "go env"
- restore_cache: # restores saved cache if no changes are detected since last run
keys:
- go-mod-v5-{{ checksum "go.sum" }}
- go-mod-v6-{{ checksum "go.sum" }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this have to change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

paths in docker executor and linux vm executor are different, so under old key dependencies was cached under wrong path. The easiest way around that is to update key version. Old key will expire after some time (I remember on github actions it was 7 days, here I am not sure)

docker:
- image: cimg/go:1.18
machine:
image: ubuntu-2204:2022.07.1
Copy link
Contributor

Choose a reason for hiding this comment

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

What installs go into this image?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah cool, I didn't know it was Circle CI specific and not some base Ubuntu. Thanks

- image: cimg/go:1.18
machine:
image: ubuntu-2204:2022.07.1
resource_class: large
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It decides on size of the machine - https://circleci.com/docs/using-linuxvm#available-linuxvm-resource-classes. Large seemed like nice mid point.

@KonradStaniec KonradStaniec merged commit 30b45f4 into main Aug 13, 2022
@KonradStaniec KonradStaniec deleted the add-step-to-ci branch August 13, 2022 05:23
vitsalis pushed a commit that referenced this pull request Jan 21, 2024
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.

2 participants