linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/11] Add Kconfig unit tests
@ 2018-03-02  4:31 Masahiro Yamada
  2018-03-02  4:31 ` [PATCH v2 01/11] kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON Masahiro Yamada
                   ` (10 more replies)
  0 siblings, 11 replies; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-02  4:31 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Michal Marek, Ulf Magnusson, Randy Dunlap,
	Luis R . Rodriguez, Masahiro Yamada, Tony Luck, linux-ia64,
	linux-kernel, Fenghua Yu


I am applying various cleanups to Kconfig these days.

However, I fear regressions.  I have been thinking of unit-tests.

There are various cryptic parts in Kconfig and corner cases where
it is difficult to notice breakage.  If unit-tests cover those,
I will be able to apply changes more confidently.

This series introduces a unit-tests.  The framework is written based
on pytest.  Also, this is written in Python 3.  Python 2 will retire
in 2020.  So, I believe new python tools should be written in Python 3.

This is my Python 3 and pytest versions.

$ python3 --version
Python 3.5.2
$ python3 -m pytest --version
This is pytest version 3.4.1, imported from /home/masahiro/.local/lib/python3.5/site-packages/pytest.py

How to use?
-----------

Please make sure Python3 and pytest are installed on your system.

Then, run "make testconfig"

The result looks like as follows:

$ make testconfig
python3 -B -m pytest ./scripts/kconfig/tests \
-o cache_dir=/home/masahiro/workspace/bsp/linux-yamada/scripts/kconfig/tests/.cache \

================================= test session starts =================================
platform linux -- Python 3.5.2, pytest-3.4.1, py-1.5.2, pluggy-0.6.0 -- /usr/bin/python3
cachedir: scripts/kconfig/tests/.cache
rootdir: /home/masahiro/workspace/bsp/linux-yamada/scripts/kconfig/tests, inifile: pytest.ini
collected 14 items

scripts/kconfig/tests/auto_submenu/__init__.py::test PASSED                     [  7%]
scripts/kconfig/tests/choice/__init__.py::test_oldask0 PASSED                   [ 14%]
scripts/kconfig/tests/choice/__init__.py::test_oldask1 PASSED                   [ 21%]
scripts/kconfig/tests/choice/__init__.py::test_allyes PASSED                    [ 28%]
scripts/kconfig/tests/choice/__init__.py::test_allmod PASSED                    [ 35%]
scripts/kconfig/tests/choice/__init__.py::test_allno PASSED                     [ 42%]
scripts/kconfig/tests/choice/__init__.py::test_alldef PASSED                    [ 50%]
scripts/kconfig/tests/choice_value_with_m_dep/__init__.py::test PASSED          [ 57%]
scripts/kconfig/tests/err_recursive_inc/__init__.py::test PASSED                [ 64%]
scripts/kconfig/tests/inter_choice/__init__.py::test PASSED                     [ 71%]
scripts/kconfig/tests/new_choice_with_dep/__init__.py::test PASSED              [ 78%]
scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py::test PASSED            [ 85%]
scripts/kconfig/tests/rand_nested_choice/__init__.py::test PASSED               [ 92%]
scripts/kconfig/tests/warn_recursive_dep/__init__.py::test PASSED               [100%]


Changes in v2:
  - Add backward compatibility for PYTHON
  - Add 'randconfig' support
  - Add docstring
  - Fix various style errors detected by 'pep8' and 'pep257'
  - coding style clean-up based on PEP8, PEP257
  - shorten directory name  'auto_submenu_creation' -> 'auto_submenu'
  - coding style clean-up based on PEP8, PEP257
  - coding style clean-up based on PEP8, PEP257
  - coding style clean-up based on PEP8, PEP257
  - coding style clean-up based on PEP8, PEP257
  - Newly added
  - Newly added
  - Fix missing end quote
  - coding style clean-up based on PEP8, PEP257
  - coding style clean-up based on PEP8, PEP257

Masahiro Yamada (11):
  kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON
  kconfig: unittest: add framework for Kconfig unit testing
  kconfig: unittest: add basic 'choice' tests
  kconfig: unittest: test automatic submenu creation
  kconfig: unittest: test if new symbols in choice are asked
  kconfig: unittest: check unneeded "is not set" with unmet dependency
  kconfig: unittest: check visibility of tri-choice values in y choice
  kconfig: unittest: test defconfig when two choices interact
  kconfig: unittest: test randconfig for choice in choice
  kconfig: unittest: test if recursive dependencies are detected
  kconfig: unittest: test if recursive inclusion is detected

 Makefile                                           |   5 +-
 arch/ia64/Makefile                                 |   2 +-
 scripts/kconfig/Makefile                           |   8 +
 scripts/kconfig/tests/auto_submenu/Kconfig         |  50 ++++
 scripts/kconfig/tests/auto_submenu/__init__.py     |  12 +
 scripts/kconfig/tests/auto_submenu/expected_stdout |  10 +
 scripts/kconfig/tests/choice/Kconfig               |  54 ++++
 scripts/kconfig/tests/choice/__init__.py           |  40 +++
 .../kconfig/tests/choice/alldef_expected_config    |   5 +
 .../kconfig/tests/choice/allmod_expected_config    |   9 +
 scripts/kconfig/tests/choice/allno_expected_config |   5 +
 .../kconfig/tests/choice/allyes_expected_config    |   9 +
 .../kconfig/tests/choice/oldask0_expected_stdout   |  10 +
 scripts/kconfig/tests/choice/oldask1_config        |   2 +
 .../kconfig/tests/choice/oldask1_expected_stdout   |  15 ++
 .../kconfig/tests/choice_value_with_m_dep/Kconfig  |  20 ++
 .../tests/choice_value_with_m_dep/__init__.py      |  15 ++
 .../kconfig/tests/choice_value_with_m_dep/config   |   2 +
 .../tests/choice_value_with_m_dep/expected_config  |   3 +
 .../tests/choice_value_with_m_dep/expected_stdout  |   4 +
 scripts/kconfig/tests/conftest.py                  | 291 +++++++++++++++++++++
 scripts/kconfig/tests/err_recursive_inc/Kconfig    |   1 +
 .../kconfig/tests/err_recursive_inc/Kconfig.inc    |   1 +
 .../kconfig/tests/err_recursive_inc/__init__.py    |  10 +
 .../tests/err_recursive_inc/expected_stderr        |   4 +
 scripts/kconfig/tests/inter_choice/Kconfig         |  24 ++
 scripts/kconfig/tests/inter_choice/__init__.py     |  14 +
 scripts/kconfig/tests/inter_choice/defconfig       |   1 +
 scripts/kconfig/tests/inter_choice/expected_config |   4 +
 scripts/kconfig/tests/new_choice_with_dep/Kconfig  |  37 +++
 .../kconfig/tests/new_choice_with_dep/__init__.py  |  14 +
 scripts/kconfig/tests/new_choice_with_dep/config   |   3 +
 .../tests/new_choice_with_dep/expected_stdout      |  10 +
 .../kconfig/tests/no_write_if_dep_unmet/Kconfig    |  14 +
 .../tests/no_write_if_dep_unmet/__init__.py        |  19 ++
 scripts/kconfig/tests/no_write_if_dep_unmet/config |   1 +
 .../tests/no_write_if_dep_unmet/expected_config    |   5 +
 scripts/kconfig/tests/pytest.ini                   |   6 +
 scripts/kconfig/tests/rand_nested_choice/Kconfig   |  33 +++
 .../kconfig/tests/rand_nested_choice/__init__.py   |  16 ++
 .../tests/rand_nested_choice/expected_stdout0      |   2 +
 .../tests/rand_nested_choice/expected_stdout1      |   4 +
 .../tests/rand_nested_choice/expected_stdout2      |   5 +
 scripts/kconfig/tests/warn_recursive_dep/Kconfig   |  62 +++++
 .../kconfig/tests/warn_recursive_dep/__init__.py   |   9 +
 .../tests/warn_recursive_dep/expected_stderr       |  30 +++
 46 files changed, 897 insertions(+), 3 deletions(-)
 create mode 100644 scripts/kconfig/tests/auto_submenu/Kconfig
 create mode 100644 scripts/kconfig/tests/auto_submenu/__init__.py
 create mode 100644 scripts/kconfig/tests/auto_submenu/expected_stdout
 create mode 100644 scripts/kconfig/tests/choice/Kconfig
 create mode 100644 scripts/kconfig/tests/choice/__init__.py
 create mode 100644 scripts/kconfig/tests/choice/alldef_expected_config
 create mode 100644 scripts/kconfig/tests/choice/allmod_expected_config
 create mode 100644 scripts/kconfig/tests/choice/allno_expected_config
 create mode 100644 scripts/kconfig/tests/choice/allyes_expected_config
 create mode 100644 scripts/kconfig/tests/choice/oldask0_expected_stdout
 create mode 100644 scripts/kconfig/tests/choice/oldask1_config
 create mode 100644 scripts/kconfig/tests/choice/oldask1_expected_stdout
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/config
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/expected_config
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
 create mode 100644 scripts/kconfig/tests/conftest.py
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/__init__.py
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/expected_stderr
 create mode 100644 scripts/kconfig/tests/inter_choice/Kconfig
 create mode 100644 scripts/kconfig/tests/inter_choice/__init__.py
 create mode 100644 scripts/kconfig/tests/inter_choice/defconfig
 create mode 100644 scripts/kconfig/tests/inter_choice/expected_config
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/Kconfig
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/__init__.py
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/config
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/expected_stdout
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/config
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
 create mode 100644 scripts/kconfig/tests/pytest.ini
 create mode 100644 scripts/kconfig/tests/rand_nested_choice/Kconfig
 create mode 100644 scripts/kconfig/tests/rand_nested_choice/__init__.py
 create mode 100644 scripts/kconfig/tests/rand_nested_choice/expected_stdout0
 create mode 100644 scripts/kconfig/tests/rand_nested_choice/expected_stdout1
 create mode 100644 scripts/kconfig/tests/rand_nested_choice/expected_stdout2
 create mode 100644 scripts/kconfig/tests/warn_recursive_dep/Kconfig
 create mode 100644 scripts/kconfig/tests/warn_recursive_dep/__init__.py
 create mode 100644 scripts/kconfig/tests/warn_recursive_dep/expected_stderr

-- 
2.7.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 01/11] kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON
  2018-03-02  4:31 [PATCH v2 00/11] Add Kconfig unit tests Masahiro Yamada
@ 2018-03-02  4:31 ` Masahiro Yamada
  2018-03-06 17:16   ` Tony Luck
  2018-03-02  4:31 ` [PATCH v2 02/11] kconfig: unittest: add framework for Kconfig unit testing Masahiro Yamada
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-02  4:31 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Michal Marek, Ulf Magnusson, Randy Dunlap,
	Luis R . Rodriguez, Masahiro Yamada, Tony Luck, linux-ia64,
	linux-kernel, Fenghua Yu

The variable 'PYTHON' allows users to specify a proper executable
name in case the default 'python' does not work.  However, this does
not address the case where both Python 2 and Python 3 scripts are
used in one source tree.

PEP 394 (https://www.python.org/dev/peps/pep-0394/) provides a
convention for Python scripts portability.  Here is a quotation:

  In order to tolerate differences across platforms, all new code
  that needs to invoke the Python interpreter should not specify
  'python', but rather should specify either 'python2' or 'python3'.
  This distinction should be made in shebangs, when invoking from a
  shell script, when invoking via the system() call, or when invoking
  in any other context.

arch/ia64/scripts/unwcheck.py is apparently written in Python 2, so
it should be invoked by 'python2'.

It is legitimate to use 'python' for scripts compatible with both
Python 2 and Python 3, but this is rare (at least I do not see the
case in kernel tree).  You do not need to make efforts to write your
scripts in that way.  Anyway, Python 2 will retire in 2020.

This commit is needed for new scripts written in Python 3.

For the backward compatibility for ia64, if 'PYTHON' is specified,
it is propagated to 'PYTHON2'.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Add backward compatibility for PYTHON

 Makefile           | 5 +++--
 arch/ia64/Makefile | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 4fb97e9..ea23d9b 100644
--- a/Makefile
+++ b/Makefile
@@ -393,7 +393,8 @@ GENKSYMS	= scripts/genksyms/genksyms
 INSTALLKERNEL  := installkernel
 DEPMOD		= /sbin/depmod
 PERL		= perl
-PYTHON		= python
+PYTHON2	       := $(if $(PYTHON), $(PYTHON), python2)
+PYTHON3	       := python3
 CHECK		= sparse
 
 CHECKFLAGS     := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \
@@ -439,7 +440,7 @@ GCC_PLUGINS_CFLAGS :=
 
 export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
 export CPP AR NM STRIP OBJCOPY OBJDUMP HOSTLDFLAGS HOST_LOADLIBES
-export MAKE LEX YACC AWK GENKSYMS INSTALLKERNEL PERL PYTHON UTS_MACHINE
+export MAKE LEX YACC AWK GENKSYMS INSTALLKERNEL PERL PYTHON2 PYTHON3 UTS_MACHINE
 export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
 
 export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS
diff --git a/arch/ia64/Makefile b/arch/ia64/Makefile
index 2dd7f51..862a2ba 100644
--- a/arch/ia64/Makefile
+++ b/arch/ia64/Makefile
@@ -75,7 +75,7 @@ vmlinux.gz: vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) $@
 
 unwcheck: vmlinux
-	-$(Q)READELF=$(READELF) $(PYTHON) $(srctree)/arch/ia64/scripts/unwcheck.py $<
+	-$(Q)READELF=$(READELF) $(PYTHON2) $(srctree)/arch/ia64/scripts/unwcheck.py $<
 
 archclean:
 	$(Q)$(MAKE) $(clean)=$(boot)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 02/11] kconfig: unittest: add framework for Kconfig unit testing
  2018-03-02  4:31 [PATCH v2 00/11] Add Kconfig unit tests Masahiro Yamada
  2018-03-02  4:31 ` [PATCH v2 01/11] kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON Masahiro Yamada
@ 2018-03-02  4:31 ` Masahiro Yamada
  2018-03-02  4:31 ` [PATCH v2 03/11] kconfig: unittest: add basic 'choice' tests Masahiro Yamada
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-02  4:31 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Michal Marek, Ulf Magnusson, Randy Dunlap,
	Luis R . Rodriguez, Masahiro Yamada, linux-kernel

Many parts in Kconfig are so cryptic and need refactoring.  However,
its complexity prevents us from moving forward.  There are several
naive corner cases where it is difficult to notice breakage.  If
those are covered by unit tests, we will be able to touch the code
with more confidence.

Here is a simple test framework based on pytest.  The conftest.py
provides a fixture useful to run commands such as 'oldaskconfig' etc.
and to compare the resulted .config, stdout, stderr with expectations.

How to add test cases?
----------------------

For each test case, you should create a subdirectory under
scripts/kconfig/tests/ (so test cases are separated from each other).
Every test case directory should contain the following files:

 - __init__.py: describes test functions
 - Kconfig: the top level Kconfig file for the test

To do a useful job, test cases generally need additional data like
input .config and information about expected results.

How to run tests?
-----------------

You need Python3 and pytest.  Then, run "make testconfig".  O= option
is supported.  If V=1 is given, detailed logs captured during tests
are displayed.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
---

Changes in v2:
  - Add 'randconfig' support
  - Add docstring
  - Fix various style errors detected by 'pep8' and 'pep257'

 scripts/kconfig/Makefile          |   8 ++
 scripts/kconfig/tests/conftest.py | 291 ++++++++++++++++++++++++++++++++++++++
 scripts/kconfig/tests/pytest.ini  |   6 +
 3 files changed, 305 insertions(+)
 create mode 100644 scripts/kconfig/tests/conftest.py
 create mode 100644 scripts/kconfig/tests/pytest.ini

diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index 753a6de4..085aab1 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -142,6 +142,14 @@ PHONY += tinyconfig
 tinyconfig:
 	$(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig tiny.config
 
+# CHECK: -o cache_dir=<path> working?
+PHONY += testconfig
+testconfig: $(obj)/conf
+	$(PYTHON3) -B -m pytest $(srctree)/$(src)/tests \
+	-o cache_dir=$(abspath $(obj)/tests/.cache) \
+	$(if $(findstring 1,$(KBUILD_VERBOSE)),--capture=no)
+clean-dirs += tests/.cache
+
 # Help text used by make help
 help:
 	@echo  '  config	  - Update current config utilising a line-oriented program'
diff --git a/scripts/kconfig/tests/conftest.py b/scripts/kconfig/tests/conftest.py
new file mode 100644
index 0000000..0345ef6
--- /dev/null
+++ b/scripts/kconfig/tests/conftest.py
@@ -0,0 +1,291 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) 2018 Masahiro Yamada <yamada.masahiro@socionext.com>
+#
+
+"""
+Kconfig unit testing framework.
+
+This provides fixture functions commonly used from test files.
+"""
+
+import os
+import pytest
+import shutil
+import subprocess
+import tempfile
+
+CONF_PATH = os.path.abspath(os.path.join('scripts', 'kconfig', 'conf'))
+
+
+class Conf:
+    """Kconfig runner and result checker.
+
+    This class provides methods to run text-based interface of Kconfig
+    (scripts/kconfig/conf) and retrieve the resulted configuration,
+    stdout, and stderr.  It also provides methods to compare those
+    results with expectations.
+    """
+
+    def __init__(self, request):
+        """Create a new Conf instance.
+
+        request: object to introspect the requesting test module
+        """
+        # the directory of the test being run
+        self._test_dir = os.path.dirname(str(request.fspath))
+
+    # runners
+    def _run_conf(self, mode, dot_config=None, out_file='.config',
+                  interactive=False, in_keys=None, extra_env={}):
+        """Run text-based Kconfig executable and save the result.
+
+        mode: input mode option (--oldaskconfig, --defconfig=<file> etc.)
+        dot_config: .config file to use for configuration base
+        out_file: file name to contain the output config data
+        interactive: flag to specify the interactive mode
+        in_keys: key inputs for interactive modes
+        extra_env: additional environments
+        returncode: exit status of the Kconfig executable
+        """
+        command = [CONF_PATH, mode, 'Kconfig']
+
+        # Override 'srctree' environment to make the test as the top directory
+        extra_env['srctree'] = self._test_dir
+
+        # Run Kconfig in a temporary directory.
+        # This directory is automatically removed when done.
+        with tempfile.TemporaryDirectory() as temp_dir:
+
+            # if .config is given, copy it to the working directory
+            if dot_config:
+                shutil.copyfile(os.path.join(self._test_dir, dot_config),
+                                os.path.join(temp_dir, '.config'))
+
+            ps = subprocess.Popen(command,
+                                  stdin=subprocess.PIPE,
+                                  stdout=subprocess.PIPE,
+                                  stderr=subprocess.PIPE,
+                                  cwd=temp_dir,
+                                  env=dict(os.environ, **extra_env))
+
+            # If input key sequence is given, feed it to stdin.
+            if in_keys:
+                ps.stdin.write(in_keys.encode('utf-8'))
+
+            while ps.poll() is None:
+                # For interactive modes such as oldaskconfig, oldconfig,
+                # send 'Enter' key until the program finishes.
+                if interactive:
+                    ps.stdin.write(b'\n')
+
+            self.retcode = ps.returncode
+            self.stdout = ps.stdout.read().decode()
+            self.stderr = ps.stderr.read().decode()
+
+            # Retrieve the resulted config data only when .config is supposed
+            # to exist.  If the command fails, the .config does not exist.
+            # 'listnewconfig' does not produce .config in the first place.
+            if self.retcode == 0 and out_file:
+                with open(os.path.join(temp_dir, out_file)) as f:
+                    self.config = f.read()
+            else:
+                self.config = None
+
+        # Logging:
+        # Pytest captures the following information by default.  In failure
+        # of tests, the captured log will be displayed.  This will be useful to
+        # figure out what has happened.
+
+        print("[command]\n{}\n".format(' '.join(command)))
+
+        print("[retcode]\n{}\n".format(self.retcode))
+
+        print("[stdout]")
+        print(self.stdout)
+
+        print("[stderr]")
+        print(self.stderr)
+
+        if self.config is not None:
+            print("[output for '{}']".format(out_file))
+            print(self.config)
+
+        return self.retcode
+
+    def oldaskconfig(self, dot_config=None, in_keys=None):
+        """Run oldaskconfig.
+
+        dot_config: .config file to use for configuration base (optional)
+        in_key: key inputs (optional)
+        returncode: exit status of the Kconfig executable
+        """
+        return self._run_conf('--oldaskconfig', dot_config=dot_config,
+                              interactive=True, in_keys=in_keys)
+
+    def oldconfig(self, dot_config=None, in_keys=None):
+        """Run oldconfig.
+
+        dot_config: .config file to use for configuration base (optional)
+        in_key: key inputs (optional)
+        returncode: exit status of the Kconfig executable
+        """
+        return self._run_conf('--oldconfig', dot_config=dot_config,
+                              interactive=True, in_keys=in_keys)
+
+    def olddefconfig(self, dot_config=None):
+        """Run olddefconfig.
+
+        dot_config: .config file to use for configuration base (optional)
+        returncode: exit status of the Kconfig executable
+        """
+        return self._run_conf('--olddefconfig', dot_config=dot_config)
+
+    def defconfig(self, defconfig):
+        """Run defconfig.
+
+        defconfig: defconfig file for input
+        returncode: exit status of the Kconfig executable
+        """
+        defconfig_path = os.path.join(self._test_dir, defconfig)
+        return self._run_conf('--defconfig={}'.format(defconfig_path))
+
+    def _allconfig(self, mode, all_config):
+        if all_config:
+            all_config_path = os.path.join(self._test_dir, all_config)
+            extra_env = {'KCONFIG_ALLCONFIG': all_config_path}
+        else:
+            extra_env = {}
+
+        return self._run_conf('--{}config'.format(mode), extra_env=extra_env)
+
+    def allyesconfig(self, all_config=None):
+        """Run allyesconfig.
+
+        all_config: fragment config file for KCONFIG_ALLCONFIG (optional)
+        returncode: exit status of the Kconfig executable
+        """
+        return self._allconfig('allyes', all_config)
+
+    def allmodconfig(self, all_config=None):
+        """Run allmodconfig.
+
+        all_config: fragment config file for KCONFIG_ALLCONFIG (optional)
+        returncode: exit status of the Kconfig executable
+        """
+        return self._allconfig('allmod', all_config)
+
+    def allnoconfig(self, all_config=None):
+        """Run allnoconfig.
+
+        all_config: fragment config file for KCONFIG_ALLCONFIG (optional)
+        returncode: exit status of the Kconfig executable
+        """
+        return self._allconfig('allno', all_config)
+
+    def alldefconfig(self, all_config=None):
+        """Run alldefconfig.
+
+        all_config: fragment config file for KCONFIG_ALLCONFIG (optional)
+        returncode: exit status of the Kconfig executable
+        """
+        return self._allconfig('alldef', all_config)
+
+    def randconfig(self, all_config=None):
+        """Run randconfig.
+
+        all_config: fragment config file for KCONFIG_ALLCONFIG (optional)
+        returncode: exit status of the Kconfig executable
+        """
+        return self._allconfig('rand', all_config)
+
+    def savedefconfig(self, dot_config):
+        """Run savedefconfig.
+
+        dot_config: .config file for input
+        returncode: exit status of the Kconfig executable
+        """
+        return self._run_conf('--savedefconfig', out_file='defconfig')
+
+    def listnewconfig(self, dot_config=None):
+        """Run listnewconfig.
+
+        dot_config: .config file to use for configuration base (optional)
+        returncode: exit status of the Kconfig executable
+        """
+        return self._run_conf('--listnewconfig', dot_config=dot_config,
+                              out_file=None)
+
+    # checkers
+    def _read_and_compare(self, compare, expected):
+        """Compare the result with expectation.
+
+        compare: function to compare the result with expectation
+        expected: file that contains the expected data
+        """
+        with open(os.path.join(self._test_dir, expected)) as f:
+            expected_data = f.read()
+        return compare(self, expected_data)
+
+    def _contains(self, attr, expected):
+        return self._read_and_compare(
+                                    lambda s, e: getattr(s, attr).find(e) >= 0,
+                                    expected)
+
+    def _matches(self, attr, expected):
+        return self._read_and_compare(lambda s, e: getattr(s, attr) == e,
+                                      expected)
+
+    def config_contains(self, expected):
+        """Check if resulted configuration contains expected data.
+
+        expected: file that contains the expected data
+        returncode: True if result contains the expected data, False otherwise
+        """
+        return self._contains('config', expected)
+
+    def config_matches(self, expected):
+        """Check if resulted configuration exactly matches expected data.
+
+        expected: file that contains the expected data
+        returncode: True if result matches the expected data, False otherwise
+        """
+        return self._matches('config', expected)
+
+    def stdout_contains(self, expected):
+        """Check if resulted stdout contains expected data.
+
+        expected: file that contains the expected data
+        returncode: True if result contains the expected data, False otherwise
+        """
+        return self._contains('stdout', expected)
+
+    def stdout_matches(self, expected):
+        """Check if resulted stdout exactly matches expected data.
+
+        expected: file that contains the expected data
+        returncode: True if result matches the expected data, False otherwise
+        """
+        return self._matches('stdout', expected)
+
+    def stderr_contains(self, expected):
+        """Check if resulted stderr contains expected data.
+
+        expected: file that contains the expected data
+        returncode: True if result contains the expected data, False otherwise
+        """
+        return self._contains('stderr', expected)
+
+    def stderr_matches(self, expected):
+        """Check if resulted stderr exactly matches expected data.
+
+        expected: file that contains the expected data
+        returncode: True if result matches the expected data, False otherwise
+        """
+        return self._matches('stderr', expected)
+
+
+@pytest.fixture(scope="module")
+def conf(request):
+    """Create a Conf instance and provide it to test functions."""
+    return Conf(request)
diff --git a/scripts/kconfig/tests/pytest.ini b/scripts/kconfig/tests/pytest.ini
new file mode 100644
index 0000000..07b94e0
--- /dev/null
+++ b/scripts/kconfig/tests/pytest.ini
@@ -0,0 +1,6 @@
+[pytest]
+addopts = --verbose
+# Pytest requires that test files have unique names, because pytest imports
+# them as top-level modules.  It is silly to prefix or suffix a test file with
+# the directory name that contains it.  Use __init__.py for all test files.
+python_files = __init__.py
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 03/11] kconfig: unittest: add basic 'choice' tests
  2018-03-02  4:31 [PATCH v2 00/11] Add Kconfig unit tests Masahiro Yamada
  2018-03-02  4:31 ` [PATCH v2 01/11] kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON Masahiro Yamada
  2018-03-02  4:31 ` [PATCH v2 02/11] kconfig: unittest: add framework for Kconfig unit testing Masahiro Yamada
@ 2018-03-02  4:31 ` Masahiro Yamada
  2018-03-02  4:31 ` [PATCH v2 04/11] kconfig: unittest: test automatic submenu creation Masahiro Yamada
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-02  4:31 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Michal Marek, Ulf Magnusson, Randy Dunlap,
	Luis R . Rodriguez, Masahiro Yamada, linux-kernel

The calculation of 'choice' is a bit complicated part in Kconfig.

The behavior of 'y' choice is intuitive.  If choice values are tristate,
the choice can be 'm' where each value can be enabled independently.
Also, if a choice is marked as 'optional', the whole choice can be
invisible.

Test basic functionality of choice.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
---

Changes in v2:
  - coding style clean-up based on PEP8, PEP257

 scripts/kconfig/tests/choice/Kconfig               | 54 ++++++++++++++++++++++
 scripts/kconfig/tests/choice/__init__.py           | 40 ++++++++++++++++
 .../kconfig/tests/choice/alldef_expected_config    |  5 ++
 .../kconfig/tests/choice/allmod_expected_config    |  9 ++++
 scripts/kconfig/tests/choice/allno_expected_config |  5 ++
 .../kconfig/tests/choice/allyes_expected_config    |  9 ++++
 .../kconfig/tests/choice/oldask0_expected_stdout   | 10 ++++
 scripts/kconfig/tests/choice/oldask1_config        |  2 +
 .../kconfig/tests/choice/oldask1_expected_stdout   | 15 ++++++
 9 files changed, 149 insertions(+)
 create mode 100644 scripts/kconfig/tests/choice/Kconfig
 create mode 100644 scripts/kconfig/tests/choice/__init__.py
 create mode 100644 scripts/kconfig/tests/choice/alldef_expected_config
 create mode 100644 scripts/kconfig/tests/choice/allmod_expected_config
 create mode 100644 scripts/kconfig/tests/choice/allno_expected_config
 create mode 100644 scripts/kconfig/tests/choice/allyes_expected_config
 create mode 100644 scripts/kconfig/tests/choice/oldask0_expected_stdout
 create mode 100644 scripts/kconfig/tests/choice/oldask1_config
 create mode 100644 scripts/kconfig/tests/choice/oldask1_expected_stdout

diff --git a/scripts/kconfig/tests/choice/Kconfig b/scripts/kconfig/tests/choice/Kconfig
new file mode 100644
index 0000000..cc60e9c
--- /dev/null
+++ b/scripts/kconfig/tests/choice/Kconfig
@@ -0,0 +1,54 @@
+config MODULES
+	bool "Enable loadable module support"
+	option modules
+	default y
+
+choice
+	prompt "boolean choice"
+	default BOOL_CHOICE1
+
+config BOOL_CHOICE0
+	bool "choice 0"
+
+config BOOL_CHOICE1
+	bool "choice 1"
+
+endchoice
+
+choice
+	prompt "optional boolean choice"
+	optional
+	default OPT_BOOL_CHOICE1
+
+config OPT_BOOL_CHOICE0
+	bool "choice 0"
+
+config OPT_BOOL_CHOICE1
+	bool "choice 1"
+
+endchoice
+
+choice
+	prompt "tristate choice"
+	default TRI_CHOICE1
+
+config TRI_CHOICE0
+	tristate "choice 0"
+
+config TRI_CHOICE1
+	tristate "choice 1"
+
+endchoice
+
+choice
+	prompt "optional tristate choice"
+	optional
+	default OPT_TRI_CHOICE1
+
+config OPT_TRI_CHOICE0
+	tristate "choice 0"
+
+config OPT_TRI_CHOICE1
+	tristate "choice 1"
+
+endchoice
diff --git a/scripts/kconfig/tests/choice/__init__.py b/scripts/kconfig/tests/choice/__init__.py
new file mode 100644
index 0000000..9edcc52
--- /dev/null
+++ b/scripts/kconfig/tests/choice/__init__.py
@@ -0,0 +1,40 @@
+"""
+Basic choice tests.
+
+The handling of 'choice' is a bit complicated part in Kconfig.
+
+The behavior of 'y' choice is intuitive.  If choice values are tristate,
+the choice can be 'm' where each value can be enabled independently.
+Also, if a choice is marked as 'optional', the whole choice can be
+invisible.
+"""
+
+
+def test_oldask0(conf):
+    assert conf.oldaskconfig() == 0
+    assert conf.stdout_contains('oldask0_expected_stdout')
+
+
+def test_oldask1(conf):
+    assert conf.oldaskconfig('oldask1_config') == 0
+    assert conf.stdout_contains('oldask1_expected_stdout')
+
+
+def test_allyes(conf):
+    assert conf.allyesconfig() == 0
+    assert conf.config_contains('allyes_expected_config')
+
+
+def test_allmod(conf):
+    assert conf.allmodconfig() == 0
+    assert conf.config_contains('allmod_expected_config')
+
+
+def test_allno(conf):
+    assert conf.allnoconfig() == 0
+    assert conf.config_contains('allno_expected_config')
+
+
+def test_alldef(conf):
+    assert conf.alldefconfig() == 0
+    assert conf.config_contains('alldef_expected_config')
diff --git a/scripts/kconfig/tests/choice/alldef_expected_config b/scripts/kconfig/tests/choice/alldef_expected_config
new file mode 100644
index 0000000..7a754bf
--- /dev/null
+++ b/scripts/kconfig/tests/choice/alldef_expected_config
@@ -0,0 +1,5 @@
+CONFIG_MODULES=y
+# CONFIG_BOOL_CHOICE0 is not set
+CONFIG_BOOL_CHOICE1=y
+# CONFIG_TRI_CHOICE0 is not set
+# CONFIG_TRI_CHOICE1 is not set
diff --git a/scripts/kconfig/tests/choice/allmod_expected_config b/scripts/kconfig/tests/choice/allmod_expected_config
new file mode 100644
index 0000000..f1f5dcd
--- /dev/null
+++ b/scripts/kconfig/tests/choice/allmod_expected_config
@@ -0,0 +1,9 @@
+CONFIG_MODULES=y
+# CONFIG_BOOL_CHOICE0 is not set
+CONFIG_BOOL_CHOICE1=y
+# CONFIG_OPT_BOOL_CHOICE0 is not set
+CONFIG_OPT_BOOL_CHOICE1=y
+CONFIG_TRI_CHOICE0=m
+CONFIG_TRI_CHOICE1=m
+CONFIG_OPT_TRI_CHOICE0=m
+CONFIG_OPT_TRI_CHOICE1=m
diff --git a/scripts/kconfig/tests/choice/allno_expected_config b/scripts/kconfig/tests/choice/allno_expected_config
new file mode 100644
index 0000000..b88ee7a
--- /dev/null
+++ b/scripts/kconfig/tests/choice/allno_expected_config
@@ -0,0 +1,5 @@
+# CONFIG_MODULES is not set
+# CONFIG_BOOL_CHOICE0 is not set
+CONFIG_BOOL_CHOICE1=y
+# CONFIG_TRI_CHOICE0 is not set
+CONFIG_TRI_CHOICE1=y
diff --git a/scripts/kconfig/tests/choice/allyes_expected_config b/scripts/kconfig/tests/choice/allyes_expected_config
new file mode 100644
index 0000000..e5a062a
--- /dev/null
+++ b/scripts/kconfig/tests/choice/allyes_expected_config
@@ -0,0 +1,9 @@
+CONFIG_MODULES=y
+# CONFIG_BOOL_CHOICE0 is not set
+CONFIG_BOOL_CHOICE1=y
+# CONFIG_OPT_BOOL_CHOICE0 is not set
+CONFIG_OPT_BOOL_CHOICE1=y
+# CONFIG_TRI_CHOICE0 is not set
+CONFIG_TRI_CHOICE1=y
+# CONFIG_OPT_TRI_CHOICE0 is not set
+CONFIG_OPT_TRI_CHOICE1=y
diff --git a/scripts/kconfig/tests/choice/oldask0_expected_stdout b/scripts/kconfig/tests/choice/oldask0_expected_stdout
new file mode 100644
index 0000000..b251bba
--- /dev/null
+++ b/scripts/kconfig/tests/choice/oldask0_expected_stdout
@@ -0,0 +1,10 @@
+Enable loadable module support (MODULES) [Y/n/?] (NEW) 
+boolean choice
+  1. choice 0 (BOOL_CHOICE0) (NEW)
+> 2. choice 1 (BOOL_CHOICE1) (NEW)
+choice[1-2?]: 
+optional boolean choice [N/y/?] (NEW) 
+tristate choice [M/y/?] (NEW) 
+  choice 0 (TRI_CHOICE0) [N/m/?] (NEW) 
+  choice 1 (TRI_CHOICE1) [N/m/?] (NEW) 
+optional tristate choice [N/m/y/?] (NEW) 
diff --git a/scripts/kconfig/tests/choice/oldask1_config b/scripts/kconfig/tests/choice/oldask1_config
new file mode 100644
index 0000000..b67bfe3
--- /dev/null
+++ b/scripts/kconfig/tests/choice/oldask1_config
@@ -0,0 +1,2 @@
+# CONFIG_MODULES is not set
+CONFIG_OPT_BOOL_CHOICE0=y
diff --git a/scripts/kconfig/tests/choice/oldask1_expected_stdout b/scripts/kconfig/tests/choice/oldask1_expected_stdout
new file mode 100644
index 0000000..c2125e9b
--- /dev/null
+++ b/scripts/kconfig/tests/choice/oldask1_expected_stdout
@@ -0,0 +1,15 @@
+Enable loadable module support (MODULES) [N/y/?] 
+boolean choice
+  1. choice 0 (BOOL_CHOICE0) (NEW)
+> 2. choice 1 (BOOL_CHOICE1) (NEW)
+choice[1-2?]: 
+optional boolean choice [Y/n/?] (NEW) 
+optional boolean choice
+> 1. choice 0 (OPT_BOOL_CHOICE0)
+  2. choice 1 (OPT_BOOL_CHOICE1) (NEW)
+choice[1-2?]: 
+tristate choice
+  1. choice 0 (TRI_CHOICE0) (NEW)
+> 2. choice 1 (TRI_CHOICE1) (NEW)
+choice[1-2?]: 
+optional tristate choice [N/y/?] 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 04/11] kconfig: unittest: test automatic submenu creation
  2018-03-02  4:31 [PATCH v2 00/11] Add Kconfig unit tests Masahiro Yamada
                   ` (2 preceding siblings ...)
  2018-03-02  4:31 ` [PATCH v2 03/11] kconfig: unittest: add basic 'choice' tests Masahiro Yamada
@ 2018-03-02  4:31 ` Masahiro Yamada
  2018-03-02  4:31 ` [PATCH v2 05/11] kconfig: unittest: test if new symbols in choice are asked Masahiro Yamada
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-02  4:31 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Michal Marek, Ulf Magnusson, Randy Dunlap,
	Luis R . Rodriguez, Masahiro Yamada, linux-kernel

If a symbols has dependency on the preceding symbol, the menu entry
should become the submenu of the preceding one, and displayed with
deeper indentation.

This is done by restructuring the menu tree in menu_finalize().
It is a bit complicated computation, so let's add a test case.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
---

Changes in v2:
  - shorten directory name  'auto_submenu_creation' -> 'auto_submenu'
  - coding style clean-up based on PEP8, PEP257

 scripts/kconfig/tests/auto_submenu/Kconfig         | 50 ++++++++++++++++++++++
 scripts/kconfig/tests/auto_submenu/__init__.py     | 12 ++++++
 scripts/kconfig/tests/auto_submenu/expected_stdout | 10 +++++
 3 files changed, 72 insertions(+)
 create mode 100644 scripts/kconfig/tests/auto_submenu/Kconfig
 create mode 100644 scripts/kconfig/tests/auto_submenu/__init__.py
 create mode 100644 scripts/kconfig/tests/auto_submenu/expected_stdout

diff --git a/scripts/kconfig/tests/auto_submenu/Kconfig b/scripts/kconfig/tests/auto_submenu/Kconfig
new file mode 100644
index 0000000..9e11502
--- /dev/null
+++ b/scripts/kconfig/tests/auto_submenu/Kconfig
@@ -0,0 +1,50 @@
+config A
+	bool "A"
+	default y
+
+config A0
+	bool "A0"
+	depends on A
+	default y
+	help
+	  This depends on A, so should be a submenu of A.
+
+config A0_0
+	bool "A1_0"
+	depends on A0
+	help
+	  Submenus are created recursively.
+	  This should be a submenu of A0.
+
+config A1
+	bool "A1"
+	depends on A
+	default y
+	help
+	  This should line up with A0.
+
+choice
+	prompt "choice"
+	depends on A1
+	help
+	  Choice should become a submenu as well.
+
+config A1_0
+	bool "A1_0"
+
+config A1_1
+	bool "A1_1"
+
+endchoice
+
+config B
+	bool "B"
+	help
+	  This is independent of A.
+
+config C
+	bool "C"
+	depends on A
+	help
+	  This depends on A, but not a consecutive item, so should not
+	  be a submenu.
diff --git a/scripts/kconfig/tests/auto_submenu/__init__.py b/scripts/kconfig/tests/auto_submenu/__init__.py
new file mode 100644
index 0000000..32e79b8
--- /dev/null
+++ b/scripts/kconfig/tests/auto_submenu/__init__.py
@@ -0,0 +1,12 @@
+"""
+Create submenu for symbols that depend on the preceding one.
+
+If a symbols has dependency on the preceding symbol, the menu entry
+should become the submenu of the preceding one, and displayed with
+deeper indentation.
+"""
+
+
+def test(conf):
+    assert conf.oldaskconfig() == 0
+    assert conf.stdout_contains('expected_stdout')
diff --git a/scripts/kconfig/tests/auto_submenu/expected_stdout b/scripts/kconfig/tests/auto_submenu/expected_stdout
new file mode 100644
index 0000000..bf5236f
--- /dev/null
+++ b/scripts/kconfig/tests/auto_submenu/expected_stdout
@@ -0,0 +1,10 @@
+A (A) [Y/n/?] (NEW) 
+  A0 (A0) [Y/n/?] (NEW) 
+    A1_0 (A0_0) [N/y/?] (NEW) 
+  A1 (A1) [Y/n/?] (NEW) 
+    choice
+    > 1. A1_0 (A1_0) (NEW)
+      2. A1_1 (A1_1) (NEW)
+    choice[1-2?]: 
+B (B) [N/y/?] (NEW) 
+C (C) [N/y/?] (NEW) 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 05/11] kconfig: unittest: test if new symbols in choice are asked
  2018-03-02  4:31 [PATCH v2 00/11] Add Kconfig unit tests Masahiro Yamada
                   ` (3 preceding siblings ...)
  2018-03-02  4:31 ` [PATCH v2 04/11] kconfig: unittest: test automatic submenu creation Masahiro Yamada
@ 2018-03-02  4:31 ` Masahiro Yamada
  2018-03-02  4:31 ` [PATCH v2 06/11] kconfig: unittest: check unneeded "is not set" with unmet dependency Masahiro Yamada
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-02  4:31 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Michal Marek, Ulf Magnusson, Randy Dunlap,
	Luis R . Rodriguez, Masahiro Yamada, linux-kernel

If new choice values are added with new dependency, and they become
visible during user configuration, oldconfig should recognize them
as (NEW), and ask the user for choice.

This issue was fixed by commit 5d09598d488f ("kconfig: fix new choices
being skipped upon config update").

This is a subtle corner case.  Add a test case to avoid breakage.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
---

Changes in v2:
  - coding style clean-up based on PEP8, PEP257

 scripts/kconfig/tests/new_choice_with_dep/Kconfig  | 37 ++++++++++++++++++++++
 .../kconfig/tests/new_choice_with_dep/__init__.py  | 14 ++++++++
 scripts/kconfig/tests/new_choice_with_dep/config   |  3 ++
 .../tests/new_choice_with_dep/expected_stdout      | 10 ++++++
 4 files changed, 64 insertions(+)
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/Kconfig
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/__init__.py
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/config
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/expected_stdout

diff --git a/scripts/kconfig/tests/new_choice_with_dep/Kconfig b/scripts/kconfig/tests/new_choice_with_dep/Kconfig
new file mode 100644
index 0000000..53ef1b8
--- /dev/null
+++ b/scripts/kconfig/tests/new_choice_with_dep/Kconfig
@@ -0,0 +1,37 @@
+config A
+	bool "A"
+	help
+	  This is a new symbol.
+
+choice
+	prompt "Choice ?"
+	depends on A
+	help
+	  "depends on A" has been newly added.
+
+config CHOICE_B
+	bool "Choice B"
+
+config CHOICE_C
+	bool "Choice C"
+	help
+	  This is a new symbol, so should be asked.
+
+endchoice
+
+choice
+	prompt "Choice2 ?"
+
+config CHOICE_D
+	bool "Choice D"
+
+config CHOICE_E
+	bool "Choice E"
+
+config CHOICE_F
+	bool "Choice F"
+	depends on A
+	help
+	  This is a new symbol, so should be asked.
+
+endchoice
diff --git a/scripts/kconfig/tests/new_choice_with_dep/__init__.py b/scripts/kconfig/tests/new_choice_with_dep/__init__.py
new file mode 100644
index 0000000..f0e0ead
--- /dev/null
+++ b/scripts/kconfig/tests/new_choice_with_dep/__init__.py
@@ -0,0 +1,14 @@
+"""
+Ask new choice values when they become visible.
+
+If new choice values are added with new dependency, and they become
+visible during user configuration, oldconfig should recognize them
+as (NEW), and ask the user for choice.
+
+Related Linux commit: 5d09598d488f081e3be23f885ed65cbbe2d073b5
+"""
+
+
+def test(conf):
+    assert conf.oldconfig('config', 'y') == 0
+    assert conf.stdout_contains('expected_stdout')
diff --git a/scripts/kconfig/tests/new_choice_with_dep/config b/scripts/kconfig/tests/new_choice_with_dep/config
new file mode 100644
index 0000000..47ef95d
--- /dev/null
+++ b/scripts/kconfig/tests/new_choice_with_dep/config
@@ -0,0 +1,3 @@
+CONFIG_CHOICE_B=y
+# CONFIG_CHOICE_D is not set
+CONFIG_CHOICE_E=y
diff --git a/scripts/kconfig/tests/new_choice_with_dep/expected_stdout b/scripts/kconfig/tests/new_choice_with_dep/expected_stdout
new file mode 100644
index 0000000..74dc0bc
--- /dev/null
+++ b/scripts/kconfig/tests/new_choice_with_dep/expected_stdout
@@ -0,0 +1,10 @@
+A (A) [N/y/?] (NEW) y
+  Choice ?
+  > 1. Choice B (CHOICE_B)
+    2. Choice C (CHOICE_C) (NEW)
+  choice[1-2?]: 
+Choice2 ?
+  1. Choice D (CHOICE_D)
+> 2. Choice E (CHOICE_E)
+  3. Choice F (CHOICE_F) (NEW)
+choice[1-3?]: 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 06/11] kconfig: unittest: check unneeded "is not set" with unmet dependency
  2018-03-02  4:31 [PATCH v2 00/11] Add Kconfig unit tests Masahiro Yamada
                   ` (4 preceding siblings ...)
  2018-03-02  4:31 ` [PATCH v2 05/11] kconfig: unittest: test if new symbols in choice are asked Masahiro Yamada
@ 2018-03-02  4:31 ` Masahiro Yamada
  2018-03-02  4:31 ` [PATCH v2 07/11] kconfig: unittest: check visibility of tri-choice values in y choice Masahiro Yamada
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-02  4:31 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Michal Marek, Ulf Magnusson, Randy Dunlap,
	Luis R . Rodriguez, Masahiro Yamada, linux-kernel

Commit cb67ab2cd2b8 ("kconfig: do not write choice values when their
dependency becomes n") fixed a problem where "# CONFIG_... is not set"
for choice values are wrongly written into the .config file when they
are once visible, then become invisible later.

Add a test for this naive case.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
---

Changes in v2:
  - coding style clean-up based on PEP8, PEP257

 scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig   | 14 ++++++++++++++
 .../kconfig/tests/no_write_if_dep_unmet/__init__.py   | 19 +++++++++++++++++++
 scripts/kconfig/tests/no_write_if_dep_unmet/config    |  1 +
 .../tests/no_write_if_dep_unmet/expected_config       |  5 +++++
 4 files changed, 39 insertions(+)
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/config
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/expected_config

diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig b/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
new file mode 100644
index 0000000..c00b8fe
--- /dev/null
+++ b/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
@@ -0,0 +1,14 @@
+config A
+	bool "A"
+
+choice
+	prompt "Choice ?"
+	depends on A
+
+config CHOICE_B
+	bool "Choice B"
+
+config CHOICE_C
+	bool "Choice C"
+
+endchoice
diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
new file mode 100644
index 0000000..207261b
--- /dev/null
+++ b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
@@ -0,0 +1,19 @@
+"""
+Do not write choice values to .config if the dependency is unmet.
+
+"# CONFIG_... is not set" should not be written into the .config file
+for symbols with unmet dependency.
+
+This was not working correctly for choice values because choice needs
+a bit different symbol computation.
+
+This checks that no unneeded "# COFIG_... is not set" is contained in
+the .config file.
+
+Related Linux commit: cb67ab2cd2b8abd9650292c986c79901e3073a59
+"""
+
+
+def test(conf):
+    assert conf.oldaskconfig('config', 'n') == 0
+    assert conf.config_matches('expected_config')
diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/config b/scripts/kconfig/tests/no_write_if_dep_unmet/config
new file mode 100644
index 0000000..abd280e
--- /dev/null
+++ b/scripts/kconfig/tests/no_write_if_dep_unmet/config
@@ -0,0 +1 @@
+CONFIG_A=y
diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
new file mode 100644
index 0000000..0d15e41
--- /dev/null
+++ b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
@@ -0,0 +1,5 @@
+#
+# Automatically generated file; DO NOT EDIT.
+# Linux Kernel Configuration
+#
+# CONFIG_A is not set
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 07/11] kconfig: unittest: check visibility of tri-choice values in y choice
  2018-03-02  4:31 [PATCH v2 00/11] Add Kconfig unit tests Masahiro Yamada
                   ` (5 preceding siblings ...)
  2018-03-02  4:31 ` [PATCH v2 06/11] kconfig: unittest: check unneeded "is not set" with unmet dependency Masahiro Yamada
@ 2018-03-02  4:31 ` Masahiro Yamada
  2018-03-02  4:31 ` [PATCH v2 08/11] kconfig: unittest: test defconfig when two choices interact Masahiro Yamada
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-02  4:31 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Michal Marek, Ulf Magnusson, Randy Dunlap,
	Luis R . Rodriguez, Masahiro Yamada, linux-kernel

If tristate choice values depend on symbols set to 'm', they should be
hidden when the choice containing them is changed from 'm' to 'y'
(i.e. exclusive choice).

This issue was fixed by commit fa64e5f6a35e ("kconfig/symbol.c: handle
choice_values that depend on 'm' symbols").

Add a test case to avoid regression.

For the input in this unit test, there is a room for argument if
"# CONFIG_CHOICE1 is not set" should be written to the .config file.

After commit fa64e5f6a35e, this line was written to the .config file.

With commit cb67ab2cd2b8 ("kconfig: do not write choice values when
their dependency becomes n"), it is not written now.

In this test, "# CONFIG_CHOICE1 is not set" is don't care.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
---

Changes in v2:
  - coding style clean-up based on PEP8, PEP257

 .../kconfig/tests/choice_value_with_m_dep/Kconfig    | 20 ++++++++++++++++++++
 .../tests/choice_value_with_m_dep/__init__.py        | 15 +++++++++++++++
 scripts/kconfig/tests/choice_value_with_m_dep/config |  2 ++
 .../tests/choice_value_with_m_dep/expected_config    |  3 +++
 .../tests/choice_value_with_m_dep/expected_stdout    |  4 ++++
 5 files changed, 44 insertions(+)
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/config
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/expected_config
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout

diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig b/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
new file mode 100644
index 0000000..dbc49e2
--- /dev/null
+++ b/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
@@ -0,0 +1,20 @@
+config MODULES
+	bool
+	default y
+	option modules
+
+config DEP
+	tristate
+	default m
+
+choice
+	prompt "Tristate Choice"
+
+config CHOICE0
+	tristate "Choice 0"
+
+config CHOICE1
+	tristate "Choice 1"
+	depends on DEP
+
+endchoice
diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py b/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
new file mode 100644
index 0000000..f8d728c
--- /dev/null
+++ b/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
@@ -0,0 +1,15 @@
+"""
+Hide tristate choice values with mod dependency in y choice.
+
+If tristate choice values depend on symbols set to 'm', they should be
+hidden when the choice containing them is changed from 'm' to 'y'
+(i.e. exclusive choice).
+
+Related Linux commit: fa64e5f6a35efd5e77d639125d973077ca506074
+"""
+
+
+def test(conf):
+    assert conf.oldaskconfig('config', 'y') == 0
+    assert conf.config_contains('expected_config')
+    assert conf.stdout_contains('expected_stdout')
diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/config b/scripts/kconfig/tests/choice_value_with_m_dep/config
new file mode 100644
index 0000000..3a126b7
--- /dev/null
+++ b/scripts/kconfig/tests/choice_value_with_m_dep/config
@@ -0,0 +1,2 @@
+CONFIG_CHOICE0=m
+CONFIG_CHOICE1=m
diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/expected_config b/scripts/kconfig/tests/choice_value_with_m_dep/expected_config
new file mode 100644
index 0000000..4d07b44
--- /dev/null
+++ b/scripts/kconfig/tests/choice_value_with_m_dep/expected_config
@@ -0,0 +1,3 @@
+CONFIG_MODULES=y
+CONFIG_DEP=m
+CONFIG_CHOICE0=y
diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout b/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
new file mode 100644
index 0000000..2b50ab6
--- /dev/null
+++ b/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
@@ -0,0 +1,4 @@
+Tristate Choice [M/y/?] y
+Tristate Choice
+> 1. Choice 0 (CHOICE0)
+choice[1]: 1
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 08/11] kconfig: unittest: test defconfig when two choices interact
  2018-03-02  4:31 [PATCH v2 00/11] Add Kconfig unit tests Masahiro Yamada
                   ` (6 preceding siblings ...)
  2018-03-02  4:31 ` [PATCH v2 07/11] kconfig: unittest: check visibility of tri-choice values in y choice Masahiro Yamada
@ 2018-03-02  4:31 ` Masahiro Yamada
  2018-03-02 10:41   ` Ulf Magnusson
  2018-03-02  4:31 ` [PATCH v2 09/11] kconfig: unittest: test randconfig for choice in choice Masahiro Yamada
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-02  4:31 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Michal Marek, Ulf Magnusson, Randy Dunlap,
	Luis R . Rodriguez, Masahiro Yamada, linux-kernel

Commit fbe98bb9ed3d ("kconfig: Fix defconfig when one choice menu
selects options that another choice menu depends on") fixed defconfig
when two choices interact (i.e. calculating the visibility of a choice
requires to calculate another choice).

The test code in that commit log was based on the real world example,
and complicated.  So, I shrunk it down to the following:

defconfig.choice:
---8<---
CONFIG_CHOICE_VAL0=y
---8<---

---8<---
config MODULES
        bool "Enable loadable module support"
        option modules
        default y

choice
        prompt "Choice"

config CHOICE_VAL0
        tristate "Choice 0"

config CHOICE_VAL1
        tristate "Choice 1"

endchoice

choice
        prompt "Another choice"
        depends on CHOICE_VAL0

config DUMMY
        bool "dummy"

endchoice
---8<---

Prior to commit fbe98bb9ed3d,

  $ scripts/kconfig/conf --defconfig=defconfig.choice Kconfig.choice

resulted in:

  CONFIG_MODULES=y
  CONFIG_CHOICE_VAL0=m
  # CONFIG_CHOICE_VAL1 is not set
  CONFIG_DUMMY=y

where the expected result would be:

  CONFIG_MODULES=y
  CONFIG_CHOICE_VAL0=y
  # CONFIG_CHOICE_VAL1 is not set
  CONFIG_DUMMY=y

Roughly, this weird behavior happened like this:

Symbols are calculated a couple of times.  First, all symbols are
calculated in conf_read().  The first 'choice' is evaluated to 'y'
due to the SYMBOL_DEF_USER flag, but sym_calc_choice() clears it
unless all of its choice values are explicitly set by the user.

conf_set_all_new_symbols() clears all SYMBOL_VALID flags.  Then, only
choices are calculated.  At this point, the SYMBOL_DEF_USER for the
first choice is unset, so, it is evaluated to 'm'.  (this is weird)
set_all_choice_values() sets SYMBOL_DEF_USER again to choice symbols.

When calculating the second choice, due to 'depends on CHOICE_VAL0',
it triggers the calculation of CHOICE_VAL0.  As a result, SYMBOL_VALID
is set for CHOICE_VAL0.

Symbols except choices get the final chance of re-calculation in
conf_write().  In a normal case, CHOICE_VAL0 would be re-caluculated,
then the first choice would be indirectly re-calculated with the
SYMBOL_DEF_USER which has been set by set_all_choice_values(), which
would be evaluated to 'y'.  But, in this case, CHOICE_VAL0 has been
marked SYMBOL_VALID, so it is simply skipped.  Then, =m is written out
to the .config file.

Add a unit test for this naive case.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Newly added

 scripts/kconfig/tests/inter_choice/Kconfig         | 24 ++++++++++++++++++++++
 scripts/kconfig/tests/inter_choice/__init__.py     | 14 +++++++++++++
 scripts/kconfig/tests/inter_choice/defconfig       |  1 +
 scripts/kconfig/tests/inter_choice/expected_config |  4 ++++
 4 files changed, 43 insertions(+)
 create mode 100644 scripts/kconfig/tests/inter_choice/Kconfig
 create mode 100644 scripts/kconfig/tests/inter_choice/__init__.py
 create mode 100644 scripts/kconfig/tests/inter_choice/defconfig
 create mode 100644 scripts/kconfig/tests/inter_choice/expected_config

diff --git a/scripts/kconfig/tests/inter_choice/Kconfig b/scripts/kconfig/tests/inter_choice/Kconfig
new file mode 100644
index 0000000..57d55c4
--- /dev/null
+++ b/scripts/kconfig/tests/inter_choice/Kconfig
@@ -0,0 +1,24 @@
+config MODULES
+	bool "Enable loadable module support"
+	option modules
+	default y
+
+choice
+	prompt "Choice"
+
+config CHOICE_VAL0
+	tristate "Choice 0"
+
+config CHOIVE_VAL1
+	tristate "Choice 1"
+
+endchoice
+
+choice
+	prompt "Another choice"
+	depends on CHOICE_VAL0
+
+config DUMMY
+	bool "dummy"
+
+endchoice
diff --git a/scripts/kconfig/tests/inter_choice/__init__.py b/scripts/kconfig/tests/inter_choice/__init__.py
new file mode 100644
index 0000000..5c7fc36
--- /dev/null
+++ b/scripts/kconfig/tests/inter_choice/__init__.py
@@ -0,0 +1,14 @@
+"""
+Do not affect user-assigned choice value by another choice.
+
+Handling of state flags for choices is complecated.  In old days,
+the defconfig result of a choice could be affected by another choice
+if those choices interact by 'depends on', 'select', etc.
+
+Related Linux commit: fbe98bb9ed3dae23e320c6b113e35f129538d14a
+"""
+
+
+def test(conf):
+    assert conf.defconfig('defconfig') == 0
+    assert conf.config_contains('expected_config')
diff --git a/scripts/kconfig/tests/inter_choice/defconfig b/scripts/kconfig/tests/inter_choice/defconfig
new file mode 100644
index 0000000..162c414
--- /dev/null
+++ b/scripts/kconfig/tests/inter_choice/defconfig
@@ -0,0 +1 @@
+CONFIG_CHOICE_VAL0=y
diff --git a/scripts/kconfig/tests/inter_choice/expected_config b/scripts/kconfig/tests/inter_choice/expected_config
new file mode 100644
index 0000000..5dceefb
--- /dev/null
+++ b/scripts/kconfig/tests/inter_choice/expected_config
@@ -0,0 +1,4 @@
+CONFIG_MODULES=y
+CONFIG_CHOICE_VAL0=y
+# CONFIG_CHOIVE_VAL1 is not set
+CONFIG_DUMMY=y
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 09/11] kconfig: unittest: test randconfig for choice in choice
  2018-03-02  4:31 [PATCH v2 00/11] Add Kconfig unit tests Masahiro Yamada
                   ` (7 preceding siblings ...)
  2018-03-02  4:31 ` [PATCH v2 08/11] kconfig: unittest: test defconfig when two choices interact Masahiro Yamada
@ 2018-03-02  4:31 ` Masahiro Yamada
  2018-03-02 11:14   ` Ulf Magnusson
  2018-03-02  4:32 ` [PATCH v2 10/11] kconfig: unittest: test if recursive dependencies are detected Masahiro Yamada
  2018-03-02  4:32 ` [PATCH v2 11/11] kconfig: unittest: test if recursive inclusion is detected Masahiro Yamada
  10 siblings, 1 reply; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-02  4:31 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Michal Marek, Ulf Magnusson, Randy Dunlap,
	Luis R . Rodriguez, Masahiro Yamada, linux-kernel

Commit 3b9a19e08960 ("kconfig: loop as long as we changed some symbols
in randconfig") fixed randconfig where a choice contains a sub-choice.
Prior to that commit, the sub-choice values were not set.

This is complicated usage, but it is still used in the real world;
drivers/usb/gadget/legacy/Kconfig is source'd in a choice context,
then creates a sub-choice in it.

For the test case in this commit, there are 3 possible results.

Case 1:
  CONFIG_A=y
  # CONFIG_B is not set

Case 2:
  # CONFIG_A is not set
  CONFIG_B=y
  CONFIG_C=y
  # CONFIG_D is not set

Case 3:
  # CONFIG_A is not set
  CONFIG_B=y
  # CONFIG_C is not set
  CONFIG_D=y
  CONFIG_E=y

So, this test iterates several times, and checks if the result is
either of the three.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Newly added

 scripts/kconfig/tests/rand_nested_choice/Kconfig   | 33 ++++++++++++++++++++++
 .../kconfig/tests/rand_nested_choice/__init__.py   | 16 +++++++++++
 .../tests/rand_nested_choice/expected_stdout0      |  2 ++
 .../tests/rand_nested_choice/expected_stdout1      |  4 +++
 .../tests/rand_nested_choice/expected_stdout2      |  5 ++++
 5 files changed, 60 insertions(+)
 create mode 100644 scripts/kconfig/tests/rand_nested_choice/Kconfig
 create mode 100644 scripts/kconfig/tests/rand_nested_choice/__init__.py
 create mode 100644 scripts/kconfig/tests/rand_nested_choice/expected_stdout0
 create mode 100644 scripts/kconfig/tests/rand_nested_choice/expected_stdout1
 create mode 100644 scripts/kconfig/tests/rand_nested_choice/expected_stdout2

diff --git a/scripts/kconfig/tests/rand_nested_choice/Kconfig b/scripts/kconfig/tests/rand_nested_choice/Kconfig
new file mode 100644
index 0000000..c591d51
--- /dev/null
+++ b/scripts/kconfig/tests/rand_nested_choice/Kconfig
@@ -0,0 +1,33 @@
+choice
+	prompt "choice"
+
+config A
+	bool "A"
+
+config B
+	bool "B"
+
+if B
+choice
+	prompt "sub choice"
+
+config C
+	bool "C"
+
+config D
+	bool "D"
+
+if D
+choice
+	prompt "subsub choice"
+
+config E
+	bool "E"
+
+endchoice
+endif # D
+
+endchoice
+endif # B
+
+endchoice
diff --git a/scripts/kconfig/tests/rand_nested_choice/__init__.py b/scripts/kconfig/tests/rand_nested_choice/__init__.py
new file mode 100644
index 0000000..9ceadf6
--- /dev/null
+++ b/scripts/kconfig/tests/rand_nested_choice/__init__.py
@@ -0,0 +1,16 @@
+"""
+Set random values resursively in netsted choices.
+
+Kconfig can create a choice-in-choice structure by using 'if' statement.
+randconfig should correctly set randome choice values.
+
+Related Linux commit: 3b9a19e08960e5cdad5253998637653e592a3c29
+"""
+
+
+def test(conf):
+    for i in range(20):
+        assert conf.randconfig() == 0
+        assert (conf.config_contains('expected_stdout0') or
+                conf.config_contains('expected_stdout1') or
+                conf.config_contains('expected_stdout2'))
diff --git a/scripts/kconfig/tests/rand_nested_choice/expected_stdout0 b/scripts/kconfig/tests/rand_nested_choice/expected_stdout0
new file mode 100644
index 0000000..05450f3
--- /dev/null
+++ b/scripts/kconfig/tests/rand_nested_choice/expected_stdout0
@@ -0,0 +1,2 @@
+CONFIG_A=y
+# CONFIG_B is not set
diff --git a/scripts/kconfig/tests/rand_nested_choice/expected_stdout1 b/scripts/kconfig/tests/rand_nested_choice/expected_stdout1
new file mode 100644
index 0000000..37ab295
--- /dev/null
+++ b/scripts/kconfig/tests/rand_nested_choice/expected_stdout1
@@ -0,0 +1,4 @@
+# CONFIG_A is not set
+CONFIG_B=y
+CONFIG_C=y
+# CONFIG_D is not set
diff --git a/scripts/kconfig/tests/rand_nested_choice/expected_stdout2 b/scripts/kconfig/tests/rand_nested_choice/expected_stdout2
new file mode 100644
index 0000000..849ff47
--- /dev/null
+++ b/scripts/kconfig/tests/rand_nested_choice/expected_stdout2
@@ -0,0 +1,5 @@
+# CONFIG_A is not set
+CONFIG_B=y
+# CONFIG_C is not set
+CONFIG_D=y
+CONFIG_E=y
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 10/11] kconfig: unittest: test if recursive dependencies are detected
  2018-03-02  4:31 [PATCH v2 00/11] Add Kconfig unit tests Masahiro Yamada
                   ` (8 preceding siblings ...)
  2018-03-02  4:31 ` [PATCH v2 09/11] kconfig: unittest: test randconfig for choice in choice Masahiro Yamada
@ 2018-03-02  4:32 ` Masahiro Yamada
  2018-03-02  4:32 ` [PATCH v2 11/11] kconfig: unittest: test if recursive inclusion is detected Masahiro Yamada
  10 siblings, 0 replies; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-02  4:32 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Michal Marek, Ulf Magnusson, Randy Dunlap,
	Luis R . Rodriguez, Masahiro Yamada, linux-kernel

Recursive dependency should be detected and warned.  Test this.

(This indirectly tests the line number increments.)

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
---

Changes in v2:
  - Fix missing end quote
  - coding style clean-up based on PEP8, PEP257

 scripts/kconfig/tests/warn_recursive_dep/Kconfig   | 62 ++++++++++++++++++++++
 .../kconfig/tests/warn_recursive_dep/__init__.py   |  9 ++++
 .../tests/warn_recursive_dep/expected_stderr       | 30 +++++++++++
 3 files changed, 101 insertions(+)
 create mode 100644 scripts/kconfig/tests/warn_recursive_dep/Kconfig
 create mode 100644 scripts/kconfig/tests/warn_recursive_dep/__init__.py
 create mode 100644 scripts/kconfig/tests/warn_recursive_dep/expected_stderr

diff --git a/scripts/kconfig/tests/warn_recursive_dep/Kconfig b/scripts/kconfig/tests/warn_recursive_dep/Kconfig
new file mode 100644
index 0000000..a65bfcb
--- /dev/null
+++ b/scripts/kconfig/tests/warn_recursive_dep/Kconfig
@@ -0,0 +1,62 @@
+# depends on itself
+
+config A
+	bool "A"
+	depends on A
+
+# select itself
+
+config B
+	bool
+	select B
+
+# depends on each other
+
+config C1
+	bool "C1"
+	depends on C2
+
+config C2
+	bool "C2"
+	depends on C1
+
+# depends on and select
+
+config D1
+	bool "D1"
+	depends on D2
+	select D2
+
+config D2
+	bool
+
+# depends on and imply
+# This is not recursive dependency
+
+config E1
+	bool "E1"
+	depends on E2
+	imply E2
+
+config E2
+	bool "E2"
+
+# property
+
+config F1
+	bool "F1"
+	default F2
+
+config F2
+	bool "F2"
+	depends on F1
+
+# menu
+
+menu "menu depending on its content"
+	depends on G
+
+config G
+	bool "G"
+
+endmenu
diff --git a/scripts/kconfig/tests/warn_recursive_dep/__init__.py b/scripts/kconfig/tests/warn_recursive_dep/__init__.py
new file mode 100644
index 0000000..adb2195
--- /dev/null
+++ b/scripts/kconfig/tests/warn_recursive_dep/__init__.py
@@ -0,0 +1,9 @@
+"""
+Warn recursive inclusion.
+
+Recursive dependency should be warned.
+"""
+
+def test(conf):
+    assert conf.oldaskconfig() == 0
+    assert conf.stderr_contains('expected_stderr')
diff --git a/scripts/kconfig/tests/warn_recursive_dep/expected_stderr b/scripts/kconfig/tests/warn_recursive_dep/expected_stderr
new file mode 100644
index 0000000..3de807d
--- /dev/null
+++ b/scripts/kconfig/tests/warn_recursive_dep/expected_stderr
@@ -0,0 +1,30 @@
+Kconfig:9:error: recursive dependency detected!
+Kconfig:9:	symbol B is selected by B
+For a resolution refer to Documentation/kbuild/kconfig-language.txt
+subsection "Kconfig recursive dependency limitations"
+
+Kconfig:3:error: recursive dependency detected!
+Kconfig:3:	symbol A depends on A
+For a resolution refer to Documentation/kbuild/kconfig-language.txt
+subsection "Kconfig recursive dependency limitations"
+
+Kconfig:15:error: recursive dependency detected!
+Kconfig:15:	symbol C1 depends on C2
+Kconfig:19:	symbol C2 depends on C1
+For a resolution refer to Documentation/kbuild/kconfig-language.txt
+subsection "Kconfig recursive dependency limitations"
+
+Kconfig:30:error: recursive dependency detected!
+Kconfig:30:	symbol D2 is selected by D1
+Kconfig:25:	symbol D1 depends on D2
+For a resolution refer to Documentation/kbuild/kconfig-language.txt
+subsection "Kconfig recursive dependency limitations"
+
+Kconfig:59:error: recursive dependency detected!
+Kconfig:59:	symbol G depends on G
+For a resolution refer to Documentation/kbuild/kconfig-language.txt
+subsection "Kconfig recursive dependency limitations"
+
+Kconfig:50:error: recursive dependency detected!
+Kconfig:50:	symbol F2 depends on F1
+Kconfig:48:	symbol F1 default value contains F2
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 11/11] kconfig: unittest: test if recursive inclusion is detected
  2018-03-02  4:31 [PATCH v2 00/11] Add Kconfig unit tests Masahiro Yamada
                   ` (9 preceding siblings ...)
  2018-03-02  4:32 ` [PATCH v2 10/11] kconfig: unittest: test if recursive dependencies are detected Masahiro Yamada
@ 2018-03-02  4:32 ` Masahiro Yamada
  2018-03-02  7:08   ` Masahiro Yamada
  10 siblings, 1 reply; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-02  4:32 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Michal Marek, Ulf Magnusson, Randy Dunlap,
	Luis R . Rodriguez, Masahiro Yamada, linux-kernel

If recursive inclusion is detected, it should fail with error messages.
Test this.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
---

Changes in v2:
  - coding style clean-up based on PEP8, PEP257

 scripts/kconfig/tests/err_recursive_inc/Kconfig         |  1 +
 scripts/kconfig/tests/err_recursive_inc/Kconfig.inc     |  1 +
 scripts/kconfig/tests/err_recursive_inc/__init__.py     | 10 ++++++++++
 scripts/kconfig/tests/err_recursive_inc/expected_stderr |  4 ++++
 4 files changed, 16 insertions(+)
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/__init__.py
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/expected_stderr

diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig b/scripts/kconfig/tests/err_recursive_inc/Kconfig
new file mode 100644
index 0000000..3ce7a3f
--- /dev/null
+++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig
@@ -0,0 +1 @@
+source "Kconfig.inc"
diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
new file mode 100644
index 0000000..1fab1c1
--- /dev/null
+++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
@@ -0,0 +1 @@
+source "Kconfig"
diff --git a/scripts/kconfig/tests/err_recursive_inc/__init__.py b/scripts/kconfig/tests/err_recursive_inc/__init__.py
new file mode 100644
index 0000000..0e4c839
--- /dev/null
+++ b/scripts/kconfig/tests/err_recursive_inc/__init__.py
@@ -0,0 +1,10 @@
+"""
+Detect recursive inclusion error.
+
+If recursive inclusion is detected, it should fail with error messages.
+"""
+
+
+def test(conf):
+    assert conf.oldaskconfig() != 0
+    assert conf.stderr_contains('expected_stderr')
diff --git a/scripts/kconfig/tests/err_recursive_inc/expected_stderr b/scripts/kconfig/tests/err_recursive_inc/expected_stderr
new file mode 100644
index 0000000..b256c91
--- /dev/null
+++ b/scripts/kconfig/tests/err_recursive_inc/expected_stderr
@@ -0,0 +1,4 @@
+Kconfig:1: recursive inclusion detected. Inclusion path:
+  current file : 'Kconfig'
+  included from: 'Kconfig.inc:1'
+  included from: 'Kconfig:3'
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 11/11] kconfig: unittest: test if recursive inclusion is detected
  2018-03-02  4:32 ` [PATCH v2 11/11] kconfig: unittest: test if recursive inclusion is detected Masahiro Yamada
@ 2018-03-02  7:08   ` Masahiro Yamada
  0 siblings, 0 replies; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-02  7:08 UTC (permalink / raw)
  To: Linux Kbuild mailing list
  Cc: Sam Ravnborg, Michal Marek, Ulf Magnusson, Randy Dunlap,
	Luis R . Rodriguez, Masahiro Yamada, Linux Kernel Mailing List

2018-03-02 13:32 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> If recursive inclusion is detected, it should fail with error messages.
> Test this.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
> ---
>
> Changes in v2:
>   - coding style clean-up based on PEP8, PEP257
>
>  scripts/kconfig/tests/err_recursive_inc/Kconfig         |  1 +
>  scripts/kconfig/tests/err_recursive_inc/Kconfig.inc     |  1 +
>  scripts/kconfig/tests/err_recursive_inc/__init__.py     | 10 ++++++++++
>  scripts/kconfig/tests/err_recursive_inc/expected_stderr |  4 ++++
>  4 files changed, 16 insertions(+)
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/__init__.py
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/expected_stderr
>
> diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig b/scripts/kconfig/tests/err_recursive_inc/Kconfig
> new file mode 100644
> index 0000000..3ce7a3f
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig
> @@ -0,0 +1 @@
> +source "Kconfig.inc"
> diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
> new file mode 100644
> index 0000000..1fab1c1
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
> @@ -0,0 +1 @@
> +source "Kconfig"
> diff --git a/scripts/kconfig/tests/err_recursive_inc/__init__.py b/scripts/kconfig/tests/err_recursive_inc/__init__.py
> new file mode 100644
> index 0000000..0e4c839
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/__init__.py
> @@ -0,0 +1,10 @@
> +"""
> +Detect recursive inclusion error.
> +
> +If recursive inclusion is detected, it should fail with error messages.
> +"""
> +
> +
> +def test(conf):
> +    assert conf.oldaskconfig() != 0
> +    assert conf.stderr_contains('expected_stderr')
> diff --git a/scripts/kconfig/tests/err_recursive_inc/expected_stderr b/scripts/kconfig/tests/err_recursive_inc/expected_stderr
> new file mode 100644
> index 0000000..b256c91
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/expected_stderr
> @@ -0,0 +1,4 @@
> +Kconfig:1: recursive inclusion detected. Inclusion path:
> +  current file : 'Kconfig'
> +  included from: 'Kconfig.inc:1'
> +  included from: 'Kconfig:3'


Currently, the line number is wrong.

The following patch should be applied first
https://patchwork.kernel.org/patch/10253541/


Then, expected data should be changed

  included from: 'Kconfig:3'

->

  included from: 'Kconfig:1'



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 08/11] kconfig: unittest: test defconfig when two choices interact
  2018-03-02  4:31 ` [PATCH v2 08/11] kconfig: unittest: test defconfig when two choices interact Masahiro Yamada
@ 2018-03-02 10:41   ` Ulf Magnusson
  2018-03-06  9:21     ` Masahiro Yamada
  0 siblings, 1 reply; 20+ messages in thread
From: Ulf Magnusson @ 2018-03-02 10:41 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Kbuild mailing list, Sam Ravnborg, Michal Marek,
	Randy Dunlap, Luis R . Rodriguez, Linux Kernel Mailing List

On Fri, Mar 2, 2018 at 5:31 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Commit fbe98bb9ed3d ("kconfig: Fix defconfig when one choice menu
> selects options that another choice menu depends on") fixed defconfig
> when two choices interact (i.e. calculating the visibility of a choice
> requires to calculate another choice).
>
> The test code in that commit log was based on the real world example,
> and complicated.  So, I shrunk it down to the following:
>
> defconfig.choice:
> ---8<---
> CONFIG_CHOICE_VAL0=y
> ---8<---
>
> ---8<---
> config MODULES
>         bool "Enable loadable module support"
>         option modules
>         default y
>
> choice
>         prompt "Choice"
>
> config CHOICE_VAL0
>         tristate "Choice 0"
>
> config CHOICE_VAL1
>         tristate "Choice 1"
>
> endchoice
>
> choice
>         prompt "Another choice"
>         depends on CHOICE_VAL0
>
> config DUMMY
>         bool "dummy"
>
> endchoice
> ---8<---
>
> Prior to commit fbe98bb9ed3d,
>
>   $ scripts/kconfig/conf --defconfig=defconfig.choice Kconfig.choice
>
> resulted in:
>
>   CONFIG_MODULES=y
>   CONFIG_CHOICE_VAL0=m
>   # CONFIG_CHOICE_VAL1 is not set
>   CONFIG_DUMMY=y
>
> where the expected result would be:
>
>   CONFIG_MODULES=y
>   CONFIG_CHOICE_VAL0=y
>   # CONFIG_CHOICE_VAL1 is not set
>   CONFIG_DUMMY=y
>
> Roughly, this weird behavior happened like this:
>
> Symbols are calculated a couple of times.  First, all symbols are
> calculated in conf_read().  The first 'choice' is evaluated to 'y'
> due to the SYMBOL_DEF_USER flag, but sym_calc_choice() clears it
> unless all of its choice values are explicitly set by the user.
>
> conf_set_all_new_symbols() clears all SYMBOL_VALID flags.  Then, only
> choices are calculated.  At this point, the SYMBOL_DEF_USER for the
> first choice is unset, so, it is evaluated to 'm'.  (this is weird)

This is because tristate choices start out in m mode btw (they have an
implicit select of 'm && <visibibility>' on them, added add the end of
menu_finalize()).

> set_all_choice_values() sets SYMBOL_DEF_USER again to choice symbols.
>
> When calculating the second choice, due to 'depends on CHOICE_VAL0',
> it triggers the calculation of CHOICE_VAL0.  As a result, SYMBOL_VALID
> is set for CHOICE_VAL0.
>
> Symbols except choices get the final chance of re-calculation in
> conf_write().  In a normal case, CHOICE_VAL0 would be re-caluculated,
> then the first choice would be indirectly re-calculated with the
> SYMBOL_DEF_USER which has been set by set_all_choice_values(), which
> would be evaluated to 'y'.  But, in this case, CHOICE_VAL0 has been
> marked SYMBOL_VALID, so it is simply skipped.  Then, =m is written out
> to the .config file.
>
> Add a unit test for this naive case.

At a high level, I think the problem is that the choice mode is
forgotten. It should be y because of the CONFIG_CHOICE_VAL0=y
assignment, but reverts back to m temporarily, and during that window
a choice symbol is evaluated and gets the wrong value.

I wonder if all this twisty code and the weird flags
(SYMBOL_NEED_SET_CHOICE_VALUES... hmm) are required. Perhaps an extra
invalidation or the like would be enough.

>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> Changes in v2:
>   - Newly added
>
>  scripts/kconfig/tests/inter_choice/Kconfig         | 24 ++++++++++++++++++++++
>  scripts/kconfig/tests/inter_choice/__init__.py     | 14 +++++++++++++
>  scripts/kconfig/tests/inter_choice/defconfig       |  1 +
>  scripts/kconfig/tests/inter_choice/expected_config |  4 ++++
>  4 files changed, 43 insertions(+)
>  create mode 100644 scripts/kconfig/tests/inter_choice/Kconfig
>  create mode 100644 scripts/kconfig/tests/inter_choice/__init__.py
>  create mode 100644 scripts/kconfig/tests/inter_choice/defconfig
>  create mode 100644 scripts/kconfig/tests/inter_choice/expected_config
>
> diff --git a/scripts/kconfig/tests/inter_choice/Kconfig b/scripts/kconfig/tests/inter_choice/Kconfig
> new file mode 100644
> index 0000000..57d55c4
> --- /dev/null
> +++ b/scripts/kconfig/tests/inter_choice/Kconfig
> @@ -0,0 +1,24 @@
> +config MODULES
> +       bool "Enable loadable module support"
> +       option modules
> +       default y
> +
> +choice
> +       prompt "Choice"
> +
> +config CHOICE_VAL0
> +       tristate "Choice 0"
> +
> +config CHOIVE_VAL1
> +       tristate "Choice 1"
> +
> +endchoice
> +
> +choice
> +       prompt "Another choice"
> +       depends on CHOICE_VAL0
> +
> +config DUMMY
> +       bool "dummy"
> +
> +endchoice
> diff --git a/scripts/kconfig/tests/inter_choice/__init__.py b/scripts/kconfig/tests/inter_choice/__init__.py
> new file mode 100644
> index 0000000..5c7fc36
> --- /dev/null
> +++ b/scripts/kconfig/tests/inter_choice/__init__.py
> @@ -0,0 +1,14 @@
> +"""
> +Do not affect user-assigned choice value by another choice.
> +
> +Handling of state flags for choices is complecated.  In old days,
> +the defconfig result of a choice could be affected by another choice
> +if those choices interact by 'depends on', 'select', etc.
> +
> +Related Linux commit: fbe98bb9ed3dae23e320c6b113e35f129538d14a
> +"""
> +
> +
> +def test(conf):
> +    assert conf.defconfig('defconfig') == 0
> +    assert conf.config_contains('expected_config')
> diff --git a/scripts/kconfig/tests/inter_choice/defconfig b/scripts/kconfig/tests/inter_choice/defconfig
> new file mode 100644
> index 0000000..162c414
> --- /dev/null
> +++ b/scripts/kconfig/tests/inter_choice/defconfig
> @@ -0,0 +1 @@
> +CONFIG_CHOICE_VAL0=y
> diff --git a/scripts/kconfig/tests/inter_choice/expected_config b/scripts/kconfig/tests/inter_choice/expected_config
> new file mode 100644
> index 0000000..5dceefb
> --- /dev/null
> +++ b/scripts/kconfig/tests/inter_choice/expected_config
> @@ -0,0 +1,4 @@
> +CONFIG_MODULES=y
> +CONFIG_CHOICE_VAL0=y
> +# CONFIG_CHOIVE_VAL1 is not set
> +CONFIG_DUMMY=y
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>

This reminded me of a bug I reported ages ago, which afaict hasn't
been fixed: https://lkml.org/lkml/2012/12/5/458 (in retrospect,
sym_clear_all_valid() is cheap).

When manually patching all defconfig files in the kernel to disable
modules and running the Kconfiglib test suite, that bug triggers for a
few defconfigs. It has previously triggered for a few unpatched
defconfig files too -- see
https://github.com/ulfalizer/Kconfiglib#notes.

I just add an extra sym_clear_all_valid() at the end of
conf_set_all_new_symbols() to fix it. It'd be worth checking if that
fixes this problem too.

Cheers,
Ulf

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 09/11] kconfig: unittest: test randconfig for choice in choice
  2018-03-02  4:31 ` [PATCH v2 09/11] kconfig: unittest: test randconfig for choice in choice Masahiro Yamada
@ 2018-03-02 11:14   ` Ulf Magnusson
       [not found]     ` <CAB=NE6V+5MK-ytS+-QBppSBLz7T4hRQV8eMA2Vv9-crcbj7NYg@mail.gmail.com>
  0 siblings, 1 reply; 20+ messages in thread
From: Ulf Magnusson @ 2018-03-02 11:14 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Kbuild mailing list, Sam Ravnborg, Michal Marek,
	Randy Dunlap, Luis R . Rodriguez, Linux Kernel Mailing List

On Fri, Mar 2, 2018 at 5:31 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Commit 3b9a19e08960 ("kconfig: loop as long as we changed some symbols
> in randconfig") fixed randconfig where a choice contains a sub-choice.
> Prior to that commit, the sub-choice values were not set.
>
> This is complicated usage, but it is still used in the real world;
> drivers/usb/gadget/legacy/Kconfig is source'd in a choice context,
> then creates a sub-choice in it.

That file is the only one that does all that weird choice stuff btw.

It's an optional choice with the choice symbols source'd from a
separate file, containing non-symbol items and symbols that end up in
implicit submenus and aren't considered choice symbols.

It's as if it was written to make use of as much obscure Kconfig stuff
as possible. :P

>
> For the test case in this commit, there are 3 possible results.
>
> Case 1:
>   CONFIG_A=y
>   # CONFIG_B is not set
>
> Case 2:
>   # CONFIG_A is not set
>   CONFIG_B=y
>   CONFIG_C=y
>   # CONFIG_D is not set
>
> Case 3:
>   # CONFIG_A is not set
>   CONFIG_B=y
>   # CONFIG_C is not set
>   CONFIG_D=y
>   CONFIG_E=y
>
> So, this test iterates several times, and checks if the result is
> either of the three.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> Changes in v2:
>   - Newly added
>
>  scripts/kconfig/tests/rand_nested_choice/Kconfig   | 33 ++++++++++++++++++++++
>  .../kconfig/tests/rand_nested_choice/__init__.py   | 16 +++++++++++
>  .../tests/rand_nested_choice/expected_stdout0      |  2 ++
>  .../tests/rand_nested_choice/expected_stdout1      |  4 +++
>  .../tests/rand_nested_choice/expected_stdout2      |  5 ++++
>  5 files changed, 60 insertions(+)
>  create mode 100644 scripts/kconfig/tests/rand_nested_choice/Kconfig
>  create mode 100644 scripts/kconfig/tests/rand_nested_choice/__init__.py
>  create mode 100644 scripts/kconfig/tests/rand_nested_choice/expected_stdout0
>  create mode 100644 scripts/kconfig/tests/rand_nested_choice/expected_stdout1
>  create mode 100644 scripts/kconfig/tests/rand_nested_choice/expected_stdout2
>
> diff --git a/scripts/kconfig/tests/rand_nested_choice/Kconfig b/scripts/kconfig/tests/rand_nested_choice/Kconfig
> new file mode 100644
> index 0000000..c591d51
> --- /dev/null
> +++ b/scripts/kconfig/tests/rand_nested_choice/Kconfig
> @@ -0,0 +1,33 @@
> +choice
> +       prompt "choice"
> +
> +config A
> +       bool "A"
> +
> +config B
> +       bool "B"
> +
> +if B
> +choice
> +       prompt "sub choice"
> +
> +config C
> +       bool "C"
> +
> +config D
> +       bool "D"
> +
> +if D
> +choice
> +       prompt "subsub choice"
> +
> +config E
> +       bool "E"
> +
> +endchoice
> +endif # D
> +
> +endchoice
> +endif # B
> +
> +endchoice
> diff --git a/scripts/kconfig/tests/rand_nested_choice/__init__.py b/scripts/kconfig/tests/rand_nested_choice/__init__.py
> new file mode 100644
> index 0000000..9ceadf6
> --- /dev/null
> +++ b/scripts/kconfig/tests/rand_nested_choice/__init__.py
> @@ -0,0 +1,16 @@
> +"""
> +Set random values resursively in netsted choices.

Two small typos: recursively, nested

> +
> +Kconfig can create a choice-in-choice structure by using 'if' statement.
> +randconfig should correctly set randome choice values.

Typo: random

> +
> +Related Linux commit: 3b9a19e08960e5cdad5253998637653e592a3c29
> +"""
> +
> +
> +def test(conf):
> +    for i in range(20):
> +        assert conf.randconfig() == 0
> +        assert (conf.config_contains('expected_stdout0') or
> +                conf.config_contains('expected_stdout1') or
> +                conf.config_contains('expected_stdout2'))
> diff --git a/scripts/kconfig/tests/rand_nested_choice/expected_stdout0 b/scripts/kconfig/tests/rand_nested_choice/expected_stdout0
> new file mode 100644
> index 0000000..05450f3
> --- /dev/null
> +++ b/scripts/kconfig/tests/rand_nested_choice/expected_stdout0
> @@ -0,0 +1,2 @@
> +CONFIG_A=y
> +# CONFIG_B is not set
> diff --git a/scripts/kconfig/tests/rand_nested_choice/expected_stdout1 b/scripts/kconfig/tests/rand_nested_choice/expected_stdout1
> new file mode 100644
> index 0000000..37ab295
> --- /dev/null
> +++ b/scripts/kconfig/tests/rand_nested_choice/expected_stdout1
> @@ -0,0 +1,4 @@
> +# CONFIG_A is not set
> +CONFIG_B=y
> +CONFIG_C=y
> +# CONFIG_D is not set
> diff --git a/scripts/kconfig/tests/rand_nested_choice/expected_stdout2 b/scripts/kconfig/tests/rand_nested_choice/expected_stdout2
> new file mode 100644
> index 0000000..849ff47
> --- /dev/null
> +++ b/scripts/kconfig/tests/rand_nested_choice/expected_stdout2
> @@ -0,0 +1,5 @@
> +# CONFIG_A is not set
> +CONFIG_B=y
> +# CONFIG_C is not set
> +CONFIG_D=y
> +CONFIG_E=y
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>

Cheers,
Ulf

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 09/11] kconfig: unittest: test randconfig for choice in choice
       [not found]     ` <CAB=NE6V+5MK-ytS+-QBppSBLz7T4hRQV8eMA2Vv9-crcbj7NYg@mail.gmail.com>
@ 2018-03-03  9:34       ` Ulf Magnusson
  2018-03-06  9:36         ` Masahiro Yamada
  0 siblings, 1 reply; 20+ messages in thread
From: Ulf Magnusson @ 2018-03-03  9:34 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Masahiro Yamada, Linux Kbuild mailing list, Sam Ravnborg,
	Michal Marek, Randy Dunlap, Linux Kernel Mailing List

On Fri, Mar 2, 2018 at 10:29 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
>
>
> On Fri, Mar 2, 2018, 3:14 AM Ulf Magnusson <ulfalizer@gmail.com> wrote:
>>
>> On Fri, Mar 2, 2018 at 5:31 AM, Masahiro Yamada
>> > This is complicated usage, but it is still used in the real world;
>> > drivers/usb/gadget/legacy/Kconfig is source'd in a choice context,
>> > then creates a sub-choice in it.
>>
>> That file is the only one that does all that weird choice stuff btw.
>>
>> It's as if it was written to make use of as much obscure Kconfig stuff
>> as possible. :P
>
>
> Can't we just use another way to describe this requirement on this file,
> with the trade-off of simplifying kconfig semantics?
>
>   Luis

I don't think changing how drivers/usb/gadget/legacy/Kconfig does
things would allow for any simplifications, unfortunately (except to
get rid of the fix tested by this patch, maybe).

Being able to have non-choice-value symbols and choices in choices is
a side effect of automatic submenu creation. Symbols and choices that
depend on the symbol before them end up in a submenu, and only the
top-level symbols in the choice are marked as choice value symbols.

I always wondered whether that was an intended feature or just
something people discovered works (it seems to work anyway...). It
feels pretty iffy to have cosmetic submenus affect behavior like that,
but playing devil's advocate, it kinda makes sense to put symbols
close to the symbols they depend on if you can get away with it.

Cheers,
Ulf

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 08/11] kconfig: unittest: test defconfig when two choices interact
  2018-03-02 10:41   ` Ulf Magnusson
@ 2018-03-06  9:21     ` Masahiro Yamada
  0 siblings, 0 replies; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-06  9:21 UTC (permalink / raw)
  To: Ulf Magnusson
  Cc: Linux Kbuild mailing list, Sam Ravnborg, Michal Marek,
	Randy Dunlap, Luis R . Rodriguez, Linux Kernel Mailing List

2018-03-02 19:41 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> On Fri, Mar 2, 2018 at 5:31 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> Commit fbe98bb9ed3d ("kconfig: Fix defconfig when one choice menu
>> selects options that another choice menu depends on") fixed defconfig
>> when two choices interact (i.e. calculating the visibility of a choice
>> requires to calculate another choice).
>>
>> The test code in that commit log was based on the real world example,
>> and complicated.  So, I shrunk it down to the following:
>>
>> defconfig.choice:
>> ---8<---
>> CONFIG_CHOICE_VAL0=y
>> ---8<---
>>
>> ---8<---
>> config MODULES
>>         bool "Enable loadable module support"
>>         option modules
>>         default y
>>
>> choice
>>         prompt "Choice"
>>
>> config CHOICE_VAL0
>>         tristate "Choice 0"
>>
>> config CHOICE_VAL1
>>         tristate "Choice 1"
>>
>> endchoice
>>
>> choice
>>         prompt "Another choice"
>>         depends on CHOICE_VAL0
>>
>> config DUMMY
>>         bool "dummy"
>>
>> endchoice
>> ---8<---
>>
>> Prior to commit fbe98bb9ed3d,
>>
>>   $ scripts/kconfig/conf --defconfig=defconfig.choice Kconfig.choice
>>
>> resulted in:
>>
>>   CONFIG_MODULES=y
>>   CONFIG_CHOICE_VAL0=m
>>   # CONFIG_CHOICE_VAL1 is not set
>>   CONFIG_DUMMY=y
>>
>> where the expected result would be:
>>
>>   CONFIG_MODULES=y
>>   CONFIG_CHOICE_VAL0=y
>>   # CONFIG_CHOICE_VAL1 is not set
>>   CONFIG_DUMMY=y
>>
>> Roughly, this weird behavior happened like this:
>>
>> Symbols are calculated a couple of times.  First, all symbols are
>> calculated in conf_read().  The first 'choice' is evaluated to 'y'
>> due to the SYMBOL_DEF_USER flag, but sym_calc_choice() clears it
>> unless all of its choice values are explicitly set by the user.
>>
>> conf_set_all_new_symbols() clears all SYMBOL_VALID flags.  Then, only
>> choices are calculated.  At this point, the SYMBOL_DEF_USER for the
>> first choice is unset, so, it is evaluated to 'm'.  (this is weird)
>
> This is because tristate choices start out in m mode btw (they have an
> implicit select of 'm && <visibibility>' on them, added add the end of
> menu_finalize()).

Ah, right.  But indeed weird to forget SYMBOL_DEF_USER.


>> set_all_choice_values() sets SYMBOL_DEF_USER again to choice symbols.
>>
>> When calculating the second choice, due to 'depends on CHOICE_VAL0',
>> it triggers the calculation of CHOICE_VAL0.  As a result, SYMBOL_VALID
>> is set for CHOICE_VAL0.
>>
>> Symbols except choices get the final chance of re-calculation in
>> conf_write().  In a normal case, CHOICE_VAL0 would be re-caluculated,
>> then the first choice would be indirectly re-calculated with the
>> SYMBOL_DEF_USER which has been set by set_all_choice_values(), which
>> would be evaluated to 'y'.  But, in this case, CHOICE_VAL0 has been
>> marked SYMBOL_VALID, so it is simply skipped.  Then, =m is written out
>> to the .config file.
>>
>> Add a unit test for this naive case.
>
> At a high level, I think the problem is that the choice mode is
> forgotten. It should be y because of the CONFIG_CHOICE_VAL0=y
> assignment, but reverts back to m temporarily, and during that window
> a choice symbol is evaluated and gets the wrong value.
>
> I wonder if all this twisty code and the weird flags
> (SYMBOL_NEED_SET_CHOICE_VALUES... hmm) are required. Perhaps an extra
> invalidation or the like would be enough.


Agree.

Probably, 5d09598d488f and fbe98bb9ed3d fixed issues in a bad way.

I believe SYMBOL_DEF_USER should be set only in
conf_read(_simple) and conf_set_all_new_symbols().

It is strange to set and clear SYMBOL_DEF_USER
while calculating symbols.




>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>> Changes in v2:
>>   - Newly added
>>
>>  scripts/kconfig/tests/inter_choice/Kconfig         | 24 ++++++++++++++++++++++
>>  scripts/kconfig/tests/inter_choice/__init__.py     | 14 +++++++++++++
>>  scripts/kconfig/tests/inter_choice/defconfig       |  1 +
>>  scripts/kconfig/tests/inter_choice/expected_config |  4 ++++
>>  4 files changed, 43 insertions(+)
>>  create mode 100644 scripts/kconfig/tests/inter_choice/Kconfig
>>  create mode 100644 scripts/kconfig/tests/inter_choice/__init__.py
>>  create mode 100644 scripts/kconfig/tests/inter_choice/defconfig
>>  create mode 100644 scripts/kconfig/tests/inter_choice/expected_config
>>
>> diff --git a/scripts/kconfig/tests/inter_choice/Kconfig b/scripts/kconfig/tests/inter_choice/Kconfig
>> new file mode 100644
>> index 0000000..57d55c4
>> --- /dev/null
>> +++ b/scripts/kconfig/tests/inter_choice/Kconfig
>> @@ -0,0 +1,24 @@
>> +config MODULES
>> +       bool "Enable loadable module support"
>> +       option modules
>> +       default y
>> +
>> +choice
>> +       prompt "Choice"
>> +
>> +config CHOICE_VAL0
>> +       tristate "Choice 0"
>> +
>> +config CHOIVE_VAL1
>> +       tristate "Choice 1"
>> +
>> +endchoice
>> +
>> +choice
>> +       prompt "Another choice"
>> +       depends on CHOICE_VAL0
>> +
>> +config DUMMY
>> +       bool "dummy"
>> +
>> +endchoice
>> diff --git a/scripts/kconfig/tests/inter_choice/__init__.py b/scripts/kconfig/tests/inter_choice/__init__.py
>> new file mode 100644
>> index 0000000..5c7fc36
>> --- /dev/null
>> +++ b/scripts/kconfig/tests/inter_choice/__init__.py
>> @@ -0,0 +1,14 @@
>> +"""
>> +Do not affect user-assigned choice value by another choice.
>> +
>> +Handling of state flags for choices is complecated.  In old days,
>> +the defconfig result of a choice could be affected by another choice
>> +if those choices interact by 'depends on', 'select', etc.
>> +
>> +Related Linux commit: fbe98bb9ed3dae23e320c6b113e35f129538d14a
>> +"""
>> +
>> +
>> +def test(conf):
>> +    assert conf.defconfig('defconfig') == 0
>> +    assert conf.config_contains('expected_config')
>> diff --git a/scripts/kconfig/tests/inter_choice/defconfig b/scripts/kconfig/tests/inter_choice/defconfig
>> new file mode 100644
>> index 0000000..162c414
>> --- /dev/null
>> +++ b/scripts/kconfig/tests/inter_choice/defconfig
>> @@ -0,0 +1 @@
>> +CONFIG_CHOICE_VAL0=y
>> diff --git a/scripts/kconfig/tests/inter_choice/expected_config b/scripts/kconfig/tests/inter_choice/expected_config
>> new file mode 100644
>> index 0000000..5dceefb
>> --- /dev/null
>> +++ b/scripts/kconfig/tests/inter_choice/expected_config
>> @@ -0,0 +1,4 @@
>> +CONFIG_MODULES=y
>> +CONFIG_CHOICE_VAL0=y
>> +# CONFIG_CHOIVE_VAL1 is not set
>> +CONFIG_DUMMY=y
>> --
>> 2.7.4
>>
>
> Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
>
> This reminded me of a bug I reported ages ago, which afaict hasn't
> been fixed: https://lkml.org/lkml/2012/12/5/458 (in retrospect,
> sym_clear_all_valid() is cheap).

Fixed by fbe98bb9ed3dae23e320c6b113e35f129538d14a
a.k.a v3.10-rc1-1-gfbe98bb

The root cause is the same.


> When manually patching all defconfig files in the kernel to disable
> modules and running the Kconfiglib test suite, that bug triggers for a
> few defconfigs. It has previously triggered for a few unpatched
> defconfig files too -- see
> https://github.com/ulfalizer/Kconfiglib#notes.
>
> I just add an extra sym_clear_all_valid() at the end of
> conf_set_all_new_symbols() to fix it. It'd be worth checking if that
> fixes this problem too.
>
> Cheers,
> Ulf
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 09/11] kconfig: unittest: test randconfig for choice in choice
  2018-03-03  9:34       ` Ulf Magnusson
@ 2018-03-06  9:36         ` Masahiro Yamada
  0 siblings, 0 replies; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-06  9:36 UTC (permalink / raw)
  To: Ulf Magnusson
  Cc: Luis R. Rodriguez, Linux Kbuild mailing list, Sam Ravnborg,
	Michal Marek, Randy Dunlap, Linux Kernel Mailing List

2018-03-03 18:34 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> On Fri, Mar 2, 2018 at 10:29 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
>>
>>
>> On Fri, Mar 2, 2018, 3:14 AM Ulf Magnusson <ulfalizer@gmail.com> wrote:
>>>
>>> On Fri, Mar 2, 2018 at 5:31 AM, Masahiro Yamada
>>> > This is complicated usage, but it is still used in the real world;
>>> > drivers/usb/gadget/legacy/Kconfig is source'd in a choice context,
>>> > then creates a sub-choice in it.
>>>
>>> That file is the only one that does all that weird choice stuff btw.
>>>
>>> It's as if it was written to make use of as much obscure Kconfig stuff
>>> as possible. :P
>>
>>
>> Can't we just use another way to describe this requirement on this file,
>> with the trade-off of simplifying kconfig semantics?
>>
>>   Luis
>
> I don't think changing how drivers/usb/gadget/legacy/Kconfig does
> things would allow for any simplifications, unfortunately (except to
> get rid of the fix tested by this patch, maybe).

We could also revert 3b9a19e08960e5cd.  :)


> Being able to have non-choice-value symbols and choices in choices is
> a side effect of automatic submenu creation. Symbols and choices that
> depend on the symbol before them end up in a submenu, and only the
> top-level symbols in the choice are marked as choice value symbols.
>
> I always wondered whether that was an intended feature or just
> something people discovered works (it seems to work anyway...). It
> feels pretty iffy to have cosmetic submenus affect behavior like that,
> but playing devil's advocate, it kinda makes sense to put symbols
> close to the symbols they depend on if you can get away with it.
>

I am happy to drop this test
if removing the seem-to-work feature will clean up the code.


Now I am accumulating test cases.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 01/11] kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON
  2018-03-02  4:31 ` [PATCH v2 01/11] kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON Masahiro Yamada
@ 2018-03-06 17:16   ` Tony Luck
  2018-03-07  2:22     ` Masahiro Yamada
  0 siblings, 1 reply; 20+ messages in thread
From: Tony Luck @ 2018-03-06 17:16 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Sam Ravnborg, Michal Marek, Ulf Magnusson,
	Randy Dunlap, Luis R . Rodriguez, linux-ia64,
	Linux Kernel Mailing List, Fenghua Yu

On Thu, Mar 1, 2018 at 8:31 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> arch/ia64/scripts/unwcheck.py is apparently written in Python 2, so
> it should be invoked by 'python2'.

I pushed the patch from Corentin Labbe to update this script to run with either
python2 or python3. Linus took it yesterday:

bd5edbe67794 ("ia64: convert unwcheck.py to python3")

-Tony

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 01/11] kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON
  2018-03-06 17:16   ` Tony Luck
@ 2018-03-07  2:22     ` Masahiro Yamada
  0 siblings, 0 replies; 20+ messages in thread
From: Masahiro Yamada @ 2018-03-07  2:22 UTC (permalink / raw)
  To: Tony Luck
  Cc: linux-kbuild, Sam Ravnborg, Michal Marek, Ulf Magnusson,
	Randy Dunlap, Luis R . Rodriguez, linux-ia64,
	Linux Kernel Mailing List, Fenghua Yu

2018-03-07 2:16 GMT+09:00 Tony Luck <tony.luck@intel.com>:
> On Thu, Mar 1, 2018 at 8:31 PM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> arch/ia64/scripts/unwcheck.py is apparently written in Python 2, so
>> it should be invoked by 'python2'.
>
> I pushed the patch from Corentin Labbe to update this script to run with either
> python2 or python3. Linus took it yesterday:
>
> bd5edbe67794 ("ia64: convert unwcheck.py to python3")
>
> -Tony

Thanks for the reminder!

Yeah, I noticed this commit yesterday.
Now, unwcheck.py seems to be compatible
with both Python 2.x and 3.x
(I have not tested it, but I understood so from the
commit log.)

I will keep 'PYTHON' and arch/ia64/Makefile as they are.

I will just add 'PYTHON2' and 'PYTHON3'.




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2018-03-07  2:23 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-02  4:31 [PATCH v2 00/11] Add Kconfig unit tests Masahiro Yamada
2018-03-02  4:31 ` [PATCH v2 01/11] kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON Masahiro Yamada
2018-03-06 17:16   ` Tony Luck
2018-03-07  2:22     ` Masahiro Yamada
2018-03-02  4:31 ` [PATCH v2 02/11] kconfig: unittest: add framework for Kconfig unit testing Masahiro Yamada
2018-03-02  4:31 ` [PATCH v2 03/11] kconfig: unittest: add basic 'choice' tests Masahiro Yamada
2018-03-02  4:31 ` [PATCH v2 04/11] kconfig: unittest: test automatic submenu creation Masahiro Yamada
2018-03-02  4:31 ` [PATCH v2 05/11] kconfig: unittest: test if new symbols in choice are asked Masahiro Yamada
2018-03-02  4:31 ` [PATCH v2 06/11] kconfig: unittest: check unneeded "is not set" with unmet dependency Masahiro Yamada
2018-03-02  4:31 ` [PATCH v2 07/11] kconfig: unittest: check visibility of tri-choice values in y choice Masahiro Yamada
2018-03-02  4:31 ` [PATCH v2 08/11] kconfig: unittest: test defconfig when two choices interact Masahiro Yamada
2018-03-02 10:41   ` Ulf Magnusson
2018-03-06  9:21     ` Masahiro Yamada
2018-03-02  4:31 ` [PATCH v2 09/11] kconfig: unittest: test randconfig for choice in choice Masahiro Yamada
2018-03-02 11:14   ` Ulf Magnusson
     [not found]     ` <CAB=NE6V+5MK-ytS+-QBppSBLz7T4hRQV8eMA2Vv9-crcbj7NYg@mail.gmail.com>
2018-03-03  9:34       ` Ulf Magnusson
2018-03-06  9:36         ` Masahiro Yamada
2018-03-02  4:32 ` [PATCH v2 10/11] kconfig: unittest: test if recursive dependencies are detected Masahiro Yamada
2018-03-02  4:32 ` [PATCH v2 11/11] kconfig: unittest: test if recursive inclusion is detected Masahiro Yamada
2018-03-02  7:08   ` Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).