Skip to content

tiltfile2: basic test for loading a docker compose manifest#850

Merged
jazzdan merged 2 commits into
masterfrom
jazzdan/dc_tiltfile_test
Dec 18, 2018
Merged

tiltfile2: basic test for loading a docker compose manifest#850
jazzdan merged 2 commits into
masterfrom
jazzdan/dc_tiltfile_test

Conversation

@jazzdan

@jazzdan jazzdan commented Dec 18, 2018

Copy link
Copy Markdown
Contributor

@maiamcc any other assertions I should add for this basic test?

I also changed the docker compose yaml path to be absolute to match how we treat other files that we read and store in memory.

@jazzdan
jazzdan requested review from maiamcc and nicks December 18, 2018 16:54
if err != nil {
return nil, err
}
absConfigPath, err := filepath.Abs(configPath)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe ospath.RealAbs, like we did elsewhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, thank you!

f := newFixture(t)
defer f.TearDown()

f.setupFoo()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is a buncha k8s-specific setup, i'm concerned it muddies the test. maybe just specifically set up the dc-specific bits you need?

f.file("Tiltfile", tf)

f.load("foo")
f.assertManifest("foo", dcYAMLPath(YAMLPath))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nice. could you add a todo for me? -- assert Dockerfile and docker-compose.yml are config files

@maiamcc

maiamcc commented Dec 18, 2018

Copy link
Copy Markdown
Contributor

hmm do we want a docker compose client that we can mock out for tests so we don't actually have to call dc?

@jazzdan

jazzdan commented Dec 18, 2018

Copy link
Copy Markdown
Contributor Author

@maiamcc in my opinion not right now- we only use docker-compose here to read the YAML, not do any execution. It's similar to how this test suite depends on kustomize. But probably in the Upper we'll want a mock, yeah.

@jazzdan
jazzdan merged commit fa1a3cd into master Dec 18, 2018
@maiamcc

maiamcc commented Dec 18, 2018

Copy link
Copy Markdown
Contributor

@maiamcc in my opinion not right now- we only use docker-compose here to read the YAML, not do any execution. It's similar to how this test suite depends on kustomize. But probably in the Upper we'll want a mock, yeah.

docker-compose config VALIDATES the config as well as reading it so it does rely on our test fixture having a valid DC setup, which is maybe not ideal / muddies the test / makes set up more of a pain? but also eh, i agree that we can wait til we need it.

@nicks
nicks deleted the jazzdan/dc_tiltfile_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.

3 participants