launchpad-reviewers team mailing list archive

[Guruprasad <mp+477795@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/charm/tests/test_charm.py b/charm/tests/test_charm.py
> index f3efcd0..bc3ed1c 100644
> --- a/charm/tests/test_charm.py
> +++ b/charm/tests/test_charm.py
> @@ -287,6 +291,7 @@ def test_make_workers(harness, fs, fp, fake_user):
>              "create-lp-tests-lxd",
>              "xenial",
>              "/var/lib/buildbot/slaves/xenial-lxd-worker",
> +            "",

Please check why we need to pass the "" here since it will not be used in the actual command any way and see if we can remove it.

>          ],
>          ["update-rc.d", "buildslave", "defaults"],
>          ["service", "buildslave", "restart"],
> @@ -303,6 +308,151 @@ def test_make_workers(harness, fs, fp, fake_user):
>      assert harness.charm._stored.ubuntu_series == {"xenial"}
>  
>  
> +def test_make_both_vanilla_ubuntu_and_flavors_workers(
> +    harness, fs, fp, fake_user
> +):
> +    fs.add_real_directory(harness.charm.charm_dir / "templates")
> +    fp.keep_last_process(True)
> +    fp.register(
> +        ["sudo", "-Hu", "buildbot", "lxc", "image", "list", "-f", "json"],
> +        stdout=json.dumps({}),
> +    )
> +    fp.register([fp.any()])
> +    harness._update_config(
> +        {
> +            "manager-host": "manager.example.com",
> +            "buildbot-password": "secret",
> +            "ubuntu-series": "focal",
> +            # focal-postgres14 should be allowed as focal
> +            # series is enabled. xenial-postgres14 should be
> +            # skipped
> +            "series-flavors": "focal-postgres14 xenial-postgres14",

Since these flavors are in addition to the vanilla flavors and we do not specify values for the vanilla flavors here, can we rename this juju config option to something like `extra-series-flavors` to make clearer that this over and above the vanilla flavors?

> +        }
> +    )
> +
> +    harness.charm._make_workers()
> +
> +    # we can only access the final status that was set
> +    assert list(fp.calls) == [
> +        [
> +            "sudo",
> +            "-Hu",
> +            "buildbot",
> +            "mkdir",
> +            "-m755",
> +            "-p",
> +            "/var/lib/buildbot/slaves",
> +        ],
> +        ["sudo", "-Hu", "buildbot", "lxc", "image", "list", "-f", "json"],
> +        [
> +            "sudo",
> +            "-Hu",
> +            "buildbot",
> +            "mkdir",
> +            "-m755",
> +            "-p",
> +            "/var/lib/buildbot/slaves/focal-lxd-worker",
> +        ],
> +        [
> +            "sudo",
> +            "-Hu",
> +            "buildbot",
> +            "git",
> +            "clone",
> +            "https://git.launchpad.net/launchpad";,
> +            "/var/lib/buildbot/slaves/focal-lxd-worker/devel",
> +        ],
> +        [
> +            "sudo",
> +            "-Hu",
> +            "buildbot",
> +            "git",
> +            "clone",
> +            "https://git.launchpad.net/lp-source-dependencies";,
> +            "/var/lib/buildbot/slaves/focal-lxd-worker/"
> +            "dependencies/download-cache",
> +        ],
> +        [
> +            "sudo",
> +            "-Hu",
> +            "buildbot",
> +            "create-lp-tests-lxd",
> +            "focal",
> +            "/var/lib/buildbot/slaves/focal-lxd-worker",
> +            "",
> +        ],
> +        [
> +            "sudo",
> +            "-Hu",
> +            "buildbot",
> +            "mkdir",
> +            "-m755",
> +            "-p",
> +            "/var/lib/buildbot/slaves/focal-postgres14-lxd-worker",
> +        ],
> +        [
> +            "sudo",
> +            "-Hu",
> +            "buildbot",
> +            "git",
> +            "clone",
> +            "https://git.launchpad.net/launchpad";,
> +            "/var/lib/buildbot/slaves/focal-postgres14-lxd-worker/devel",
> +        ],
> +        [
> +            "sudo",
> +            "-Hu",
> +            "buildbot",
> +            "git",
> +            "clone",
> +            "https://git.launchpad.net/lp-source-dependencies";,
> +            "/var/lib/buildbot/slaves/focal-postgres14-lxd-worker/"
> +            "dependencies/download-cache",
> +        ],
> +        [
> +            "sudo",
> +            "-Hu",
> +            "buildbot",
> +            "create-lp-tests-lxd",
> +            "focal",
> +            "/var/lib/buildbot/slaves/focal-postgres14-lxd-worker",
> +            "postgres14",
> +        ],
> +        ["update-rc.d", "buildslave", "defaults"],
> +        ["service", "buildslave", "restart"],
> +    ]
> +    buildbot_tac = (
> +        Path("/var/lib/buildbot/slaves/buildbot.tac").read_text().splitlines()
> +    )
> +    assert "basedir = '/var/lib/buildbot/slaves'" in buildbot_tac
> +    assert "manager_host = 'manager.example.com'" in buildbot_tac
> +    assert "manager_port = 9989" in buildbot_tac
> +    assert "password = 'secret'" in buildbot_tac
> +    buildslave = Path("/etc/default/buildslave").read_text()
> +    assert 'SLAVE_BASEDIR[1]="/var/lib/buildbot/slaves"' in buildslave
> +    assert harness.charm._stored.ubuntu_series == {"focal"}
> +
> +
> +def test_raise_error_workers(harness, fs, fp, fake_user):

Can you rename the test to make it clear what is getting tested here?

> +    fs.add_real_directory(harness.charm.charm_dir / "templates")
> +    fp.keep_last_process(True)
> +    fp.register(
> +        ["sudo", "-Hu", "buildbot", "lxc", "image", "list", "-f", "json"],
> +        stdout=json.dumps({}),
> +    )
> +    fp.register([fp.any()])
> +    harness._update_config(
> +        {
> +            "manager-host": "manager.example.com",
> +            "buildbot-password": "secret",
> +            "ubuntu-series": "focal",
> +            "series-flavors": "postgres14",
> +        }
> +    )
> +
> +    pytest.raises(InvalidFlavorValueFormat, harness.charm._make_workers)
> +
> +
>  def test_make_workers_deletes_obsolete_workers(harness, fs, fp, fake_user):
>      fs.add_real_directory(harness.charm.charm_dir / "templates")
>      fp.keep_last_process(True)


-- 
https://code.launchpad.net/~tushar5526/lpbuildbot-worker/+git/lpbuildbot-worker/+merge/477795
Your team Launchpad code reviewers is requested to review the proposed merge of ~tushar5526/lpbuildbot-worker:add-support-for-creating-base-image-flavors-for-testing into lpbuildbot-worker:main.