Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

tests: Wrap or remove deprecated functions #760

Merged
merged 4 commits into from
Oct 30, 2024
Merged

tests: Wrap or remove deprecated functions #760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
265949d
tests: Remove deprecated calls to read_config
pdgendt Oct 26, 2024
7542a19
tests: Wrap or remove deprecated functions in test_config.py
pdgendt Oct 22, 2024
5da5735
configuration: Update arguments type for delete_config
pdgendt Oct 23, 2024
21a4d02
ci: tox: Turn pytest warnings into errors
pdgendt Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ jobs:
- name: install tox
run: pip3 install tox
- name: tox
run: tox -c 'check out'
run: tox -c 'check out' -- -W error
4 changes: 2 additions & 2 deletions src/west/configuration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
from pathlib import PureWindowsPath, Path
import platform
from enum import Enum
from typing import Any, Dict, Iterable, List, Optional, Tuple, TYPE_CHECKING
from typing import Any, Dict, Iterable, List, Optional, Tuple, TYPE_CHECKING, Union
import warnings

from west.util import WEST_DIR, west_dir, WestNotFound, PathType
Expand Down Expand Up @@ -491,7 +491,7 @@ def update_config(section: str, key: str, value: Any,
config.write(f)

def delete_config(section: str, key: str,
configfile: Optional[ConfigFile] = None,
configfile: Union[Optional[ConfigFile], List[ConfigFile]] = None,
topdir: Optional[PathType] = None) -> None:
'''Delete the option section.key from the given file or files.

Expand Down
1 change: 0 additions & 1 deletion tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,6 @@ def west_init_tmpdir(repos_tmpdir):
manifest = repos_tmpdir / 'repos' / 'zephyr'
cmd(['init', '-m', str(manifest), str(west_tmpdir)])
west_tmpdir.chdir()
config.read_config()
return west_tmpdir

@pytest.fixture
Expand Down
118 changes: 70 additions & 48 deletions tests/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@
import configparser
import os
import pathlib
from typing import Any, Optional
import subprocess

import pytest

from west import configuration as config
from west.util import PathType

from conftest import cmd

Expand All @@ -30,9 +32,23 @@ def autouse_config_tmpdir(config_tmpdir):
def cfg(f=ALL, topdir=None):
# Load a fresh configuration object at the given level, and return it.
cp = configparser.ConfigParser(allow_no_value=True)
config.read_config(configfile=f, config=cp, topdir=topdir)
# TODO: convert this mechanism without the global deprecated read_config
with pytest.deprecated_call():
config.read_config(configfile=f, config=cp, topdir=topdir)
return cp

def update_testcfg(section: str, key: str, value: Any,
configfile: config.ConfigFile = LOCAL,
topdir: Optional[PathType] = None) -> None:
c = config.Configuration(topdir)
c.set(option=f'{section}.{key}', value=value, configfile=configfile)

def delete_testcfg(section: str, key: str,
configfile: Optional[config.ConfigFile] = None,
topdir: Optional[PathType] = None) -> None:
c = config.Configuration(topdir)
c.delete(option=f'{section}.{key}', configfile=configfile)

def test_config_global():
# Set a global config option via the command interface. Make sure
# it can be read back using the API calls and at the command line
Expand Down Expand Up @@ -100,28 +116,28 @@ def test_config_local():
def test_config_system():
# Basic test of system-level configuration.

config.update_config('pytest', 'key', 'val', configfile=SYSTEM)
update_testcfg('pytest', 'key', 'val', configfile=SYSTEM)
assert cfg(f=ALL)['pytest']['key'] == 'val'
assert cfg(f=SYSTEM)['pytest']['key'] == 'val'
assert 'pytest' not in cfg(f=GLOBAL)
assert 'pytest' not in cfg(f=LOCAL)

config.update_config('pytest', 'key', 'val2', configfile=SYSTEM)
update_testcfg('pytest', 'key', 'val2', configfile=SYSTEM)
assert cfg(f=SYSTEM)['pytest']['key'] == 'val2'

def test_config_system_precedence():
# Test precedence rules, including system level.

config.update_config('pytest', 'key', 'sys', configfile=SYSTEM)
update_testcfg('pytest', 'key', 'sys', configfile=SYSTEM)
assert cfg(f=SYSTEM)['pytest']['key'] == 'sys'
assert cfg(f=ALL)['pytest']['key'] == 'sys'

config.update_config('pytest', 'key', 'glb', configfile=GLOBAL)
update_testcfg('pytest', 'key', 'glb', configfile=GLOBAL)
assert cfg(f=SYSTEM)['pytest']['key'] == 'sys'
assert cfg(f=GLOBAL)['pytest']['key'] == 'glb'
assert cfg(f=ALL)['pytest']['key'] == 'glb'

config.update_config('pytest', 'key', 'lcl', configfile=LOCAL)
update_testcfg('pytest', 'key', 'lcl', configfile=LOCAL)
assert cfg(f=SYSTEM)['pytest']['key'] == 'sys'
assert cfg(f=GLOBAL)['pytest']['key'] == 'glb'
assert cfg(f=LOCAL)['pytest']['key'] == 'lcl'
Expand All @@ -135,7 +151,7 @@ def test_system_creation():
assert not os.path.isfile(config._location(GLOBAL))
assert not os.path.isfile(config._location(LOCAL))

config.update_config('pytest', 'key', 'val', configfile=SYSTEM)
update_testcfg('pytest', 'key', 'val', configfile=SYSTEM)

assert os.path.isfile(config._location(SYSTEM))
assert not os.path.isfile(config._location(GLOBAL))
Expand All @@ -152,7 +168,7 @@ def test_global_creation():
assert not os.path.isfile(config._location(GLOBAL))
assert not os.path.isfile(config._location(LOCAL))

config.update_config('pytest', 'key', 'val', configfile=GLOBAL)
update_testcfg('pytest', 'key', 'val', configfile=GLOBAL)

assert not os.path.isfile(config._location(SYSTEM))
assert os.path.isfile(config._location(GLOBAL))
Expand All @@ -169,7 +185,7 @@ def test_local_creation():
assert not os.path.isfile(config._location(GLOBAL))
assert not os.path.isfile(config._location(LOCAL))

config.update_config('pytest', 'key', 'val', configfile=LOCAL)
update_testcfg('pytest', 'key', 'val', configfile=LOCAL)

assert not os.path.isfile(config._location(SYSTEM))
assert not os.path.isfile(config._location(GLOBAL))
Expand Down Expand Up @@ -203,8 +219,7 @@ def test_local_creation_with_topdir():
del os.environ['WEST_CONFIG_LOCAL']

# We should be able to write into our topdir's config file now.
config.update_config('pytest', 'key', 'val', configfile=LOCAL,
topdir=str(topdir))
update_testcfg('pytest', 'key', 'val', configfile=LOCAL, topdir=str(topdir))
assert not system.exists()
assert not glbl.exists()
assert not local.exists()
Expand All @@ -218,112 +233,119 @@ def test_local_creation_with_topdir():
def test_delete_basic():
# Basic deletion test: write local, verify global and system deletions
# don't work, then delete local does work.
config.update_config('pytest', 'key', 'val', configfile=LOCAL)
update_testcfg('pytest', 'key', 'val', configfile=LOCAL)
assert cfg(f=ALL)['pytest']['key'] == 'val'
with pytest.raises(KeyError):
config.delete_config('pytest', 'key', configfile=SYSTEM)
delete_testcfg('pytest', 'key', configfile=SYSTEM)
with pytest.raises(KeyError):
config.delete_config('pytest', 'key', configfile=GLOBAL)
config.delete_config('pytest', 'key', configfile=LOCAL)
delete_testcfg('pytest', 'key', configfile=GLOBAL)
delete_testcfg('pytest', 'key', configfile=LOCAL)
assert 'pytest' not in cfg(f=ALL)

def test_delete_all():
# Deleting ConfigFile.ALL should delete from everywhere.
config.update_config('pytest', 'key', 'system', configfile=SYSTEM)
config.update_config('pytest', 'key', 'global', configfile=GLOBAL)
config.update_config('pytest', 'key', 'local', configfile=LOCAL)
update_testcfg('pytest', 'key', 'system', configfile=SYSTEM)
update_testcfg('pytest', 'key', 'global', configfile=GLOBAL)
update_testcfg('pytest', 'key', 'local', configfile=LOCAL)
assert cfg(f=SYSTEM)['pytest']['key'] == 'system'
assert cfg(f=GLOBAL)['pytest']['key'] == 'global'
assert cfg(f=LOCAL)['pytest']['key'] == 'local'
config.delete_config('pytest', 'key', configfile=ALL)
delete_testcfg('pytest', 'key', configfile=ALL)
assert 'pytest' not in cfg(f=ALL)

def test_delete_none():
# Deleting None should delete from lowest-precedence global or
# local file only.
config.update_config('pytest', 'key', 'system', configfile=SYSTEM)
config.update_config('pytest', 'key', 'global', configfile=GLOBAL)
config.update_config('pytest', 'key', 'local', configfile=LOCAL)
# Only supported with the deprecated call
update_testcfg('pytest', 'key', 'system', configfile=SYSTEM)
update_testcfg('pytest', 'key', 'global', configfile=GLOBAL)
update_testcfg('pytest', 'key', 'local', configfile=LOCAL)
assert cfg(f=SYSTEM)['pytest']['key'] == 'system'
assert cfg(f=GLOBAL)['pytest']['key'] == 'global'
assert cfg(f=LOCAL)['pytest']['key'] == 'local'
config.delete_config('pytest', 'key', configfile=None)
delete_testcfg('pytest', 'key', configfile=None)
assert cfg(f=ALL)['pytest']['key'] == 'global'
config.delete_config('pytest', 'key', configfile=None)
delete_testcfg('pytest', 'key', configfile=None)
assert cfg(f=ALL)['pytest']['key'] == 'system'
with pytest.raises(KeyError):
with pytest.raises(KeyError), pytest.deprecated_call():
config.delete_config('pytest', 'key', configfile=None)

# Using the Configuration Class this does remove from system
delete_testcfg('pytest', 'key', configfile=None)
assert 'pytest' not in cfg(f=ALL)

def test_delete_list():
# Test delete of a list of places.
config.update_config('pytest', 'key', 'system', configfile=SYSTEM)
config.update_config('pytest', 'key', 'global', configfile=GLOBAL)
config.update_config('pytest', 'key', 'local', configfile=LOCAL)
# Only supported with the deprecated call
update_testcfg('pytest', 'key', 'system', configfile=SYSTEM)
update_testcfg('pytest', 'key', 'global', configfile=GLOBAL)
update_testcfg('pytest', 'key', 'local', configfile=LOCAL)
assert cfg(f=SYSTEM)['pytest']['key'] == 'system'
assert cfg(f=GLOBAL)['pytest']['key'] == 'global'
assert cfg(f=LOCAL)['pytest']['key'] == 'local'
config.delete_config('pytest', 'key', configfile=[GLOBAL, LOCAL])
with pytest.deprecated_call():
config.delete_config('pytest', 'key', configfile=[GLOBAL, LOCAL])
assert cfg(f=SYSTEM)['pytest']['key'] == 'system'
assert 'pytest' not in cfg(f=GLOBAL)
assert 'pytest' not in cfg(f=LOCAL)

def test_delete_system():
# Test SYSTEM-only delete.
config.update_config('pytest', 'key', 'system', configfile=SYSTEM)
config.update_config('pytest', 'key', 'global', configfile=GLOBAL)
config.update_config('pytest', 'key', 'local', configfile=LOCAL)
update_testcfg('pytest', 'key', 'system', configfile=SYSTEM)
update_testcfg('pytest', 'key', 'global', configfile=GLOBAL)
update_testcfg('pytest', 'key', 'local', configfile=LOCAL)
assert cfg(f=SYSTEM)['pytest']['key'] == 'system'
assert cfg(f=GLOBAL)['pytest']['key'] == 'global'
assert cfg(f=LOCAL)['pytest']['key'] == 'local'
config.delete_config('pytest', 'key', configfile=SYSTEM)
delete_testcfg('pytest', 'key', configfile=SYSTEM)
assert 'pytest' not in cfg(f=SYSTEM)
assert cfg(f=GLOBAL)['pytest']['key'] == 'global'
assert cfg(f=LOCAL)['pytest']['key'] == 'local'

def test_delete_global():
# Test GLOBAL-only delete.
config.update_config('pytest', 'key', 'system', configfile=SYSTEM)
config.update_config('pytest', 'key', 'global', configfile=GLOBAL)
config.update_config('pytest', 'key', 'local', configfile=LOCAL)
update_testcfg('pytest', 'key', 'system', configfile=SYSTEM)
update_testcfg('pytest', 'key', 'global', configfile=GLOBAL)
update_testcfg('pytest', 'key', 'local', configfile=LOCAL)
assert cfg(f=SYSTEM)['pytest']['key'] == 'system'
assert cfg(f=GLOBAL)['pytest']['key'] == 'global'
assert cfg(f=LOCAL)['pytest']['key'] == 'local'
config.delete_config('pytest', 'key', configfile=GLOBAL)
delete_testcfg('pytest', 'key', configfile=GLOBAL)
assert cfg(f=SYSTEM)['pytest']['key'] == 'system'
assert 'pytest' not in cfg(f=GLOBAL)
assert cfg(f=LOCAL)['pytest']['key'] == 'local'

def test_delete_local():
# Test LOCAL-only delete.
config.update_config('pytest', 'key', 'system', configfile=SYSTEM)
config.update_config('pytest', 'key', 'global', configfile=GLOBAL)
config.update_config('pytest', 'key', 'local', configfile=LOCAL)
update_testcfg('pytest', 'key', 'system', configfile=SYSTEM)
update_testcfg('pytest', 'key', 'global', configfile=GLOBAL)
update_testcfg('pytest', 'key', 'local', configfile=LOCAL)
assert cfg(f=SYSTEM)['pytest']['key'] == 'system'
assert cfg(f=GLOBAL)['pytest']['key'] == 'global'
assert cfg(f=LOCAL)['pytest']['key'] == 'local'
config.delete_config('pytest', 'key', configfile=LOCAL)
delete_testcfg('pytest', 'key', configfile=LOCAL)
assert cfg(f=SYSTEM)['pytest']['key'] == 'system'
assert cfg(f=GLOBAL)['pytest']['key'] == 'global'
assert 'pytest' not in cfg(f=LOCAL)

def test_delete_local_with_topdir():
# Test LOCAL-only delete with specified topdir.
config.update_config('pytest', 'key', 'system', configfile=SYSTEM)
config.update_config('pytest', 'key', 'global', configfile=GLOBAL)
config.update_config('pytest', 'key', 'local', configfile=LOCAL)
update_testcfg('pytest', 'key', 'system', configfile=SYSTEM)
update_testcfg('pytest', 'key', 'global', configfile=GLOBAL)
update_testcfg('pytest', 'key', 'local', configfile=LOCAL)
assert cfg(f=SYSTEM)['pytest']['key'] == 'system'
assert cfg(f=GLOBAL)['pytest']['key'] == 'global'
assert cfg(f=LOCAL)['pytest']['key'] == 'local'
config.delete_config('pytest', 'key', configfile=LOCAL)
delete_testcfg('pytest', 'key', configfile=LOCAL)
assert cfg(f=SYSTEM)['pytest']['key'] == 'system'
assert cfg(f=GLOBAL)['pytest']['key'] == 'global'
assert 'pytest' not in cfg(f=LOCAL)

def test_delete_local_one():
# Test LOCAL-only delete of one option doesn't affect the other.
config.update_config('pytest', 'key1', 'foo', configfile=LOCAL)
config.update_config('pytest', 'key2', 'bar', configfile=LOCAL)
config.delete_config('pytest', 'key1', configfile=LOCAL)
update_testcfg('pytest', 'key1', 'foo', configfile=LOCAL)
update_testcfg('pytest', 'key2', 'bar', configfile=LOCAL)
delete_testcfg('pytest', 'key1', configfile=LOCAL)
assert 'pytest' in cfg(f=LOCAL)
assert cfg(f=LOCAL)['pytest']['key2'] == 'bar'

Expand Down
5 changes: 0 additions & 5 deletions tests/test_project.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import pytest

from west import configuration as config
from west.manifest import Manifest, ManifestProject, Project, \
ManifestImportFailed
from west.manifest import ImportFlag as MIF
Expand Down Expand Up @@ -1631,7 +1630,6 @@ def test_init_local_with_manifest_filename(repos_tmpdir):
# success
cmd(['init', '--mf', 'project.yml', '-l', zephyr_install_dir])
workspace.chdir()
config.read_config()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be a separate commit?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved to a separate commit.

cmd('update')


Expand Down Expand Up @@ -1823,7 +1821,6 @@ def test_init_with_manifest_filename(repos_tmpdir):
# success
cmd(['init', '-m', manifest, '--mf', 'project.yml', west_tmpdir])
west_tmpdir.chdir()
config.read_config()
cmd('update')

def test_init_with_manifest_in_subdir(repos_tmpdir):
Expand Down Expand Up @@ -1915,7 +1912,6 @@ def do_run(self, args, ignored):
zephyr = repos_tmpdir / 'repos' / 'zephyr'
cmd(['init', '-m', zephyr, west_tmpdir])
west_tmpdir.chdir()
config.read_config()
cmd('update')

# The newline shenanigans are for Windows.
Expand Down Expand Up @@ -1995,7 +1991,6 @@ def do_run(self, args, ignored):
zephyr = repos_tmpdir / 'repos' / 'zephyr'
cmd(['init', '-m', zephyr, west_tmpdir])
west_tmpdir.chdir()
config.read_config()
cmd('update')

actual = cmd('test-extension', stderr=subprocess.STDOUT).splitlines()
Expand Down