launchpad-reviewers team mailing list archive

[Tushar Gupta <mp+477795@xxxxxxxxxxxxxxxxxx>]

Addressed the review comments. 

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",
> +            "",

Agreed. Updated the code to build the cmd array conditionally.

>          ],
>          ["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",

Yes makes sense.

> +        }
> +    )
> +
> +    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):

Sorry forgot to give it a meaningful name while local dev. Updated it now.

> +    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.