sts-sponsors team mailing list archive

[Alberto Donato <mp+435548@xxxxxxxxxxxxxxxxxx>]

Review: Approve

+1


minor comments inline

Diff comments:

> diff --git a/src/maasperf/tests/maasserver/dns/conftest.py b/src/maasperf/tests/maasserver/dns/conftest.py
> new file mode 100644
> index 0000000..1d79d25
> --- /dev/null
> +++ b/src/maasperf/tests/maasserver/dns/conftest.py
> @@ -0,0 +1,30 @@
> +# Copyright 2023 Canonical Ltd.  This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +import os
> +from tempfile import mkdtemp
> +
> +from pytest import fixture
> +
> +from provisioningserver.testing.bindfixture import BINDServer
> +
> +
> +@fixture()

nitpick: I'd perfer we always imported the pytest module and just use @pytest.fixture as a decorator, since it's a bit more explicit

> +def dns_config_path():
> +    path = mkdtemp(prefix="maas-dns-config")
> +    os.environ["MAAS_BIND_CONFIG_DIR"] = path

please use the monkeypatch fixture setenv method (https://docs.pytest.org/en/7.1.x/reference/reference.html#pytest.MonkeyPatch.setenv) to set env vars for a test.

> +    return path
> +
> +
> +@fixture()
> +def zone_file_config_path():
> +    path = mkdtemp(prefix="maas-zonefile-config")
> +    os.environ["MAAS_ZONE_FILE_CONFIG_DIR"] = path

same here

> +    return path
> +
> +
> +@fixture()
> +def bind_server():
> +    bind = BINDServer()
> +    bind.setUp()
> +    return bind


-- 
https://code.launchpad.net/~cgrabowski/maas/+git/maas/+merge/435548
Your team MAAS Committers is subscribed to branch maas:master.