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

engine: docker compose logs test #866

Merged
merged 3 commits into from
Dec 20, 2018
Merged

Conversation

jazzdan
Copy link
Contributor

@jazzdan jazzdan commented Dec 20, 2018

No description provided.

@jazzdan jazzdan requested review from nicks and maiamcc December 20, 2018 17:18
)

const pod1 = k8s.PodID("pod1")

var image1 = container.MustParseNamedTagged("re.po/project/myapp:tilt-936a185caaa266bb")

const digest1 = "sha256:936a185caaa266bb9cbe981e9e05cb78cd732b0b3280eb944412bb6f8f8f07af"

type containerBaDFixture struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is unused afaict

Copy link
Contributor

Choose a reason for hiding this comment

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

¯_(ツ)_/¯ cool! then yes delete it!

@@ -96,6 +104,9 @@ func (b *fakeBuildAndDeployer) BuildAndDeploy(ctx context.Context, manifest mode
return store.BuildResult{}, err
}

if manifest.IsDockerCompose() {
return b.nextBuildResult(imageID), nil
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is kind of hacky. Any better ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeahhh kind of hacky. fine for now tho -- except that docker compose builds don't return any info in the BuildResult (cuz there's not an image id) -- so maybe change b.nextBuildResult to work okay given a nil imageID.

)

const pod1 = k8s.PodID("pod1")

var image1 = container.MustParseNamedTagged("re.po/project/myapp:tilt-936a185caaa266bb")

const digest1 = "sha256:936a185caaa266bb9cbe981e9e05cb78cd732b0b3280eb944412bb6f8f8f07af"

type containerBaDFixture struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

¯_(ツ)_/¯ cool! then yes delete it!

@@ -2030,6 +2069,31 @@ func (f *testFixture) WriteConfigFiles(args ...string) {
f.store.Dispatch(manifestFilesChangedAction{manifestName: ConfigsManifestName, files: filenames})
}

func (f *testFixture) setupHAProxy() model.Manifest {
Copy link
Contributor

Choose a reason for hiding this comment

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

consider renaming this method to be obvious that we're setting up "some docker-compose test junk" instead of like, an actual proxy doing actual things.

also, instead of tracking origWD and reading then writing files, you could just store the file contents strings -- see testyaml.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have had a TON of problems with indentations when storing YAML that way so I am trying to sidestep it for now

func TestDockerComposeRecordsLogs(t *testing.T) {
f := newTestFixture(t)
m := f.setupHAProxy()
expected := "spoonerisms_1 | 2018-12-20T16:11:04.070480042Z yarn install v1.10."
Copy link
Contributor

Choose a reason for hiding this comment

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

...spoonerisms? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

also wait i'm confused, is this actually running docker-compose up??!

Copy link
Contributor

Choose a reason for hiding this comment

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

nvm i see! if you wanna be extra clear, make this bit its own paragraph with comment (like "call to docker-compose logs will return this line -- make sure log watcher picks it up")

also -- test that it ends up in manifest...logs? (Or todo for that test.)

@@ -96,6 +104,9 @@ func (b *fakeBuildAndDeployer) BuildAndDeploy(ctx context.Context, manifest mode
return store.BuildResult{}, err
}

if manifest.IsDockerCompose() {
return b.nextBuildResult(imageID), nil
Copy link
Contributor

Choose a reason for hiding this comment

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

yeahhh kind of hacky. fine for now tho -- except that docker compose builds don't return any info in the BuildResult (cuz there's not an image id) -- so maybe change b.nextBuildResult to work okay given a nil imageID.

Copy link
Contributor

@maiamcc maiamcc left a comment

Choose a reason for hiding this comment

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

👍

@jazzdan jazzdan merged commit 1cb41c8 into master Dec 20, 2018
@nicks nicks deleted the dmiller/ch982/dcc_log_test branch March 31, 2021 16:58
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