Put functional tests in tests/functional

I think it is bad for these reasons when `tests/` contains a mix of
functional and integration tests

 - Concepts is harder to understand, the documentation makes a good
   unit vs functional vs integration distinction, but when the
   integration tests are just two subdirs within `tests/` this is not
   clear.

 - Source filtering in the `flake.nix` is more complex. We need to
   filter out some of the dirs from `tests/`, rather than simply pick
   the dirs we want and take all of them. This is a good sign the
   structure of what we are trying to do is not matching the structure
   of the files.

With this change we have a clean:
```shell-session
$ git show 'HEAD:tests'
tree HEAD:tests

functional/
installer/
nixos/
```

.github/labeler.yml

@@ -20,4 +20,4 @@
   # Unit tests
   - src/*/tests/**/*
   # Functional and integration tests
-  - tests/**/*
+  - tests/functional/**/*

.gitignore

@@ -79,24 +79,24 @@ perl/Makefile.config
 
 /src/build-remote/build-remote
 
-# /tests/
-/tests/test-tmp
-/tests/common/vars-and-functions.sh
-/tests/result*
-/tests/restricted-innocent
-/tests/shell
-/tests/shell.drv
-/tests/config.nix
-/tests/ca/config.nix
-/tests/dyn-drv/config.nix
-/tests/repl-result-out
-/tests/test-libstoreconsumer/test-libstoreconsumer
-
-# /tests/lang/
-/tests/lang/*.out
-/tests/lang/*.out.xml
-/tests/lang/*.err
-/tests/lang/*.ast
+# /tests/functional/
+/tests/functional/test-tmp
+/tests/functional/common/vars-and-functions.sh
+/tests/functional/result*
+/tests/functional/restricted-innocent
+/tests/functional/shell
+/tests/functional/shell.drv
+/tests/functional/config.nix
+/tests/functional/ca/config.nix
+/tests/functional/dyn-drv/config.nix
+/tests/functional/repl-result-out
+/tests/functional/test-libstoreconsumer/test-libstoreconsumer
+
+# /tests/functional/lang/
+/tests/functional/lang/*.out
+/tests/functional/lang/*.out.xml
+/tests/functional/lang/*.err
+/tests/functional/lang/*.ast
 
 /perl/lib/Nix/Config.pm
 /perl/lib/Nix/Store.cc

CONTRIBUTING.md

@@ -57,7 +57,7 @@ Check out the [security policy](https://github.com/NixOS/nix/security/policy).
 
    - [ ] Fixes an [idea approved](https://github.com/NixOS/nix/labels/idea%20approved) issue
    - [ ] Tests, as appropriate:
-     - Functional tests – [`tests/**.sh`](./tests)
+     - Functional tests – [`tests/functional/**.sh`](./tests/functional)
      - Unit tests – [`src/*/tests`](./src/)
      - Integration tests – [`tests/nixos/*`](./tests/nixos)
    - [ ] User documentation in the [manual](..doc/manual/src)

Makefile

@@ -27,11 +27,11 @@ makefiles += \
   src/libutil/tests/local.mk \
   src/libstore/tests/local.mk \
   src/libexpr/tests/local.mk \
-  tests/local.mk \
-  tests/ca/local.mk \
-  tests/dyn-drv/local.mk \
-  tests/test-libstoreconsumer/local.mk \
-  tests/plugins/local.mk
+  tests/functional/local.mk \
+  tests/functional/ca/local.mk \
+  tests/functional/dyn-drv/local.mk \
+  tests/functional/test-libstoreconsumer/local.mk \
+  tests/functional/plugins/local.mk
 else
 makefiles += \
   mk/disable-tests.mk

doc/manual/src/contributing/testing.md

@@ -82,7 +82,7 @@ The characterization tests will mark themselves "skipped" since they regenerated
 
 ## Functional tests
 
-The functional tests reside under the `tests` directory and are listed in `tests/local.mk`.
+The functional tests reside under the `tests/functional` directory and are listed in `tests/functional/local.mk`.
 Each test is a bash script.
 
 ### Running the whole test suite
@@ -91,23 +91,23 @@ The whole test suite can be run with:
 
 ```shell-session
 $ make install && make installcheck
-ran test tests/foo.sh... [PASS]
-ran test tests/bar.sh... [PASS]
+ran test tests/functional/foo.sh... [PASS]
+ran test tests/functional/bar.sh... [PASS]
 ...
 ```
 
 ### Grouping tests
 
 Sometimes it is useful to group related tests so they can be easily run together without running the entire test suite.
 Each test group is in a subdirectory of `tests`.
-For example, `tests/ca/local.mk` defines a `ca` test group for content-addressed derivation outputs.
+For example, `tests/functional/ca/local.mk` defines a `ca` test group for content-addressed derivation outputs.
 
 That test group can be run like this:
 
 ```shell-session
 $ make ca.test-group -j50
-ran test tests/ca/nix-run.sh... [PASS]
-ran test tests/ca/import-derivation.sh... [PASS]
+ran test tests/functional/ca/nix-run.sh... [PASS]
+ran test tests/functional/ca/import-derivation.sh... [PASS]
 ...
 ```
 
@@ -126,21 +126,21 @@ install-tests-groups += $(test-group-name)
 Individual tests can be run with `make`:
 
 ```shell-session
-$ make tests/${testName}.sh.test
-ran test tests/${testName}.sh... [PASS]
+$ make tests/functional/${testName}.sh.test
+ran test tests/functional/${testName}.sh... [PASS]
 ```
 
 or without `make`:
 
 ```shell-session
-$ ./mk/run-test.sh tests/${testName}.sh
-ran test tests/${testName}.sh... [PASS]
+$ ./mk/run-test.sh tests/functional/${testName}.sh
+ran test tests/functional/${testName}.sh... [PASS]
 ```
 
 To see the complete output, one can also run:
 
 ```shell-session
-$ ./mk/debug-test.sh tests/${testName}.sh
+$ ./mk/debug-test.sh tests/functional/${testName}.sh
 + foo
 output from foo
 + bar
@@ -175,7 +175,7 @@ edit it like so:
 Then, running the test with `./mk/debug-test.sh` will drop you into GDB once the script reaches that point:
 
 ```shell-session
-$ ./mk/debug-test.sh tests/${testName}.sh
+$ ./mk/debug-test.sh tests/functional/${testName}.sh
 ...
 + gdb blash blub
 GNU gdb (GDB) 12.1
@@ -206,7 +206,7 @@ It is frequently useful to regenerate the expected output.
 To do that, rerun the failed test(s) with `_NIX_TEST_ACCEPT=1`.
 For example:
 ```bash
-_NIX_TEST_ACCEPT=1 make tests/lang.sh.test
+_NIX_TEST_ACCEPT=1 make tests/functional/lang.sh.test
 ```
 This convention is shared with the [characterization unit tests](#characterization-testing-1) too.
 

flake.nix

@@ -61,36 +61,28 @@
 
       nixSrc = fileset.toSource {
         root = ./.;
-        fileset = fileset.intersect baseFiles (
-          fileset.difference
-            (fileset.unions [
-              ./.version
-              ./boehmgc-coroutine-sp-fallback.diff
-              ./bootstrap.sh
-              ./configure.ac
-              ./doc
-              ./local.mk
-              ./m4
-              ./Makefile
-              ./Makefile.config.in
-              ./misc
-              ./mk
-              ./precompiled-headers.h
-              ./src
-              ./tests
-              ./unit-test-data
-              ./COPYING
-              ./scripts/local.mk
-              (fileset.fileFilter (f: lib.strings.hasPrefix "nix-profile" f.name) ./scripts)
-              # TODO: do we really need README.md? It doesn't seem used in the build.
-              ./README.md
-            ])
-            (fileset.unions [
-              # Removed file sets
-              ./tests/nixos
-              ./tests/installer
-            ])
-        );
+        fileset = fileset.intersect baseFiles (fileset.unions [
+          ./.version
+          ./boehmgc-coroutine-sp-fallback.diff
+          ./bootstrap.sh
+          ./configure.ac
+          ./doc
+          ./local.mk
+          ./m4
+          ./Makefile
+          ./Makefile.config.in
+          ./misc
+          ./mk
+          ./precompiled-headers.h
+          ./src
+          ./tests/functional
+          ./unit-test-data
+          ./COPYING
+          ./scripts/local.mk
+          (fileset.fileFilter (f: lib.strings.hasPrefix "nix-profile" f.name) ./scripts)
+          # TODO: do we really need README.md? It doesn't seem used in the build.
+          ./README.md
+        ]);
       };
 
       # Memoize nixpkgs for different platforms for efficiency.

mk/common-test.sh

@@ -1,11 +1,15 @@
+test_dir=tests/functional
+
+test=$(echo -n "$test" | sed -e "s|^$test_dir/||")
+
 TESTS_ENVIRONMENT=("TEST_NAME=${test%.*}" 'NIX_REMOTE=')
 
 : ${BASH:=/usr/bin/env bash}
 
 init_test () {
-   cd tests && env "${TESTS_ENVIRONMENT[@]}" $BASH -e init.sh 2>/dev/null > /dev/null
+   cd "$test_dir" && env "${TESTS_ENVIRONMENT[@]}" $BASH -e init.sh 2>/dev/null > /dev/null
 }
 
 run_test_proper () {
-   cd $(dirname $test) && env "${TESTS_ENVIRONMENT[@]}" $BASH -e $(basename $test)
+   cd "$test_dir/$(dirname $test)" && env "${TESTS_ENVIRONMENT[@]}" $BASH -e $(basename $test)
 }

src/libstore/gc.cc

@@ -776,7 +776,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
         }
     };
 
-    /* Synchronisation point for testing, see tests/gc-concurrent.sh. */
+    /* Synchronisation point for testing, see tests/functional/gc-concurrent.sh. */
     if (auto p = getEnv("_NIX_TEST_GC_SYNC"))
         readFile(*p);
 

src/libutil/url-parts.hh

@@ -30,7 +30,7 @@ extern std::regex refRegex;
 
 /// Instead of defining what a good Git Ref is, we define what a bad Git Ref is
 /// This is because of the definition of a ref in refs.c in https://github.com/git/git
-/// See tests/fetchGitRefs.sh for the full definition
+/// See tests/functional/fetchGitRefs.sh for the full definition
 const static std::string badGitRefRegexS = "//|^[./]|/\\.|\\.\\.|[[:cntrl:][:space:]:?^~\[]|\\\\|\\*|\\.lock$|\\.lock/|@\\{|[/.]$|^@$|^$";
 extern std::regex badGitRefRegex;
 

tests/bash-profile.sh → tests/functional/bash-profile.sh

@@ -1,6 +1,6 @@
 source common.sh
 
-sed -e "s|@localstatedir@|$TEST_ROOT/profile-var|g" -e "s|@coreutils@|$coreutils|g" < ../scripts/nix-profile.sh.in > $TEST_ROOT/nix-profile.sh
+sed -e "s|@localstatedir@|$TEST_ROOT/profile-var|g" -e "s|@coreutils@|$coreutils|g" < ../../scripts/nix-profile.sh.in > $TEST_ROOT/nix-profile.sh
 
 user=$(whoami)
 rm -rf $TEST_HOME $TEST_ROOT/profile-var

tests/build-remote-trustless.sh → tests/functional/build-remote-trustless.sh

@@ -6,7 +6,7 @@ unset NIX_STATE_DIR
 
 remoteDir=$TEST_ROOT/remote
 
-# Note: ssh{-ng}://localhost bypasses ssh. See tests/build-remote.sh for
+# Note: ssh{-ng}://localhost bypasses ssh. See tests/functional/build-remote.sh for
 # more details.
 nix-build $file -o $TEST_ROOT/result --max-jobs 0 \
   --arg busybox $busybox \

tests/ca/local.mk → tests/functional/ca/local.mk

@@ -25,4 +25,4 @@ clean-files += \
   $(d)/config.nix
 
 test-deps += \
-  tests/ca/config.nix
+  tests/functional/ca/config.nix

tests/common/vars-and-functions.sh.in → tests/functional/common/vars-and-functions.sh.in

@@ -6,7 +6,7 @@ COMMON_VARS_AND_FUNCTIONS_SH_SOURCED=1
 
 export PS4='+(${BASH_SOURCE[0]-$0}:$LINENO) '
 
-export TEST_ROOT=$(realpath ${TMPDIR:-/tmp}/nix-test)/${TEST_NAME:-default}
+export TEST_ROOT=$(realpath ${TMPDIR:-/tmp}/nix-test)/${TEST_NAME:-default/tests\/functional//}
 export NIX_STORE_DIR
 if ! NIX_STORE_DIR=$(readlink -f $TEST_ROOT/store 2> /dev/null); then
     # Maybe the build directory is symlinked.

tests/dyn-drv/local.mk → tests/functional/dyn-drv/local.mk

@@ -12,4 +12,4 @@ clean-files += \
   $(d)/config.nix
 
 test-deps += \
-  tests/dyn-drv/config.nix
+  tests/functional/dyn-drv/config.nix

tests/lang.sh → tests/functional/lang.sh

@@ -134,7 +134,7 @@ else
         echo ''
         echo 'You can rerun this test with:'
         echo ''
-        echo '    _NIX_TEST_ACCEPT=1 make tests/lang.sh.test'
+        echo '    _NIX_TEST_ACCEPT=1 make tests/functional/lang.sh.test'
         echo ''
         echo 'to regenerate the files containing the expected output,'
         echo 'and then view the git diff to decide whether a change is'