Skip to content

Ensure overriding test vars with env vars works for booleans#727

Merged
OddBloke merged 2 commits into
canonical:masterfrom
TheRealFalcon:env-vars
Dec 15, 2020
Merged

Ensure overriding test vars with env vars works for booleans#727
OddBloke merged 2 commits into
canonical:masterfrom
TheRealFalcon:env-vars

Conversation

@TheRealFalcon
Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon commented Dec 14, 2020 •
edited
Loading

Proposed Commit Message

Ensure overriding test vars with env vars works for booleans

Additional Context

n/a

Test Steps

CLOUD_INIT_KEEP_INSTANCE=False pytest <any_test>
Ensure the instance isn't kept around.

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

mitechie
mitechie approved these changes Dec 14, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@mitechie mitechie left a comment

Choose a reason for hiding this comment

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

one question on just doing all the right things for processing.

Comment thread tests/integration_tests/integration_settings.py Outdated
@mitechie
Copy link
Copy Markdown
Contributor

mitechie commented Dec 14, 2020

It'd be good to note the test/change that drove this as well to help justify the change in practice.

@TheRealFalcon
Copy link
Copy Markdown
Contributor Author

TheRealFalcon commented Dec 14, 2020

It'd be good to note the test/change that drove this as well to help justify the change in practice.

It's a fix for a test running feature that hasn't been widely used yet. The only specific thing that drove it was I was about to run a fairly large set of integration tests so wanted to make sure KEEP_INSTANCE was set to false in the environment. I started setting the env var and then remembered we didn't do any particular boolean parsing so went and fixed it real quick.

@TheRealFalcon TheRealFalcon changed the title Ensure overriding test vars with env vars works for booleans and None Ensure overriding test vars with env vars works for booleans Dec 14, 2020
OddBloke
OddBloke approved these changes Dec 15, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@OddBloke OddBloke left a comment

Choose a reason for hiding this comment

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

Thanks!

(I wonder if we have any cloud-init code that could be simplified with this? Probably not unless the interface is bug-compatible.)

Comment thread tests/integration_tests/integration_settings.py
# This file is part of cloud-init. See LICENSE file for license information.
import os

from distutils.util import strtobool
Copy link
Copy Markdown
Collaborator

@OddBloke OddBloke Dec 15, 2020

Choose a reason for hiding this comment

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

🎶 hello util my old friend 🎶

@OddBloke OddBloke merged commit 2af3f6d into canonical:master Dec 15, 2020
@TheRealFalcon TheRealFalcon deleted the env-vars branch December 15, 2020 17:06
This was referenced May 12, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@OddBloke OddBloke OddBloke approved these changes

@mitechie mitechie mitechie approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@TheRealFalcon @mitechie @OddBloke