Linux: functional test and XDG configuration support #422
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrisd8088
force-pushed
the
linux-initial-func-tests
branch
from
428715b to
883f15b
Compare
chrisd8088
force-pushed
the
linux-initial-func-tests
branch
2 times, most recently
from
acddfab to
2b20bbe
Compare
From microsoft/VFSForGit@8f4079a and microsoft/VFSForGit@ef3201b. Co-authored-by: Ashe Connor <ashe@kivikakk.ee>
We convert the MacOnly functional test category into POSIXOnly, which we also execute on Linux. From microsoft/VFSForGit@f47f68c.
chrisd8088
force-pushed
the
linux-initial-func-tests
branch
from
2b20bbe to
d5d4ad7
Compare
chrisd8088
force-pushed
the
linux-initial-func-tests
branch
from
1d25861 to
b2851b5
Compare
We adjust the name of the MacCreateHardLink() wrapper for the link(2) syscall to be POSIXCreateHardLink(), as this also runs on Linux. We also create distinct MacChmod() and LinuxChmod() wrappers for the chmod(2) syscall as the size of the mode argument varies between platforms. From microsoft/VFSForGit@79ef572.
chrisd8088
force-pushed
the
linux-initial-func-tests
branch
from
b2851b5 to
ce988df
Compare
We add the functional test run and cleanup scripts for Linux, which are derived from the Mac equivalents. We may use these manually, or in our CI jobs. See also microsoft/VFSForGit@ef3201b.
Akin to how the Windows RunFunctionalTests.bat script supports running the tests against a recent build of the Scalar binaries, we do the same in the Linux functional test script by copying the necessary binaries into the directory where the Scalar.FunctionalTests binary was published, unless the --test-scalar-on-path option was given to the script. This change permits manual runs of the functional test suite on Linux immediately after rebuilding the binaries.
We also add the Linux GVFS Git package to the list of package references in the Scalar.Common.csproj file.
chrisd8088
force-pushed
the
linux-initial-func-tests
branch
from
ce988df to
94ab823
Compare
To provide partial support for the Linux XDG Base Directory
Specification, and in particular to locate our cache, config,
and data directories for Scalar under Linux-standard locations
such as ~/.cache/scalar, we introduce a common
TryGetEnvironmentVariableBasePath() method in the shared
POSIXPlatform, which we then use as appropriate for each
type of user-specific directory.
This method accepts a list of pairs of environment variables
and path components, which are checked in sequence until a
non-empty environment variable is found; its value is then
returned with the corresponding path components appended.
Thus we can try to return "$XDG_DATA_HOME/scalar", but if that
variable is undefined or empty, return "$HOME/.local/scalar"
instead.
We also update the relevant Mac and Linux methods and accessors
to utilize TryGetEnvironmentVariableBasePath() with the
appropriate pairs of environment variables and paths. On macOS,
we update the TryGetDefaultLocalCacheRoot() and
GetDataRootForScalarImplementation() methods; on Linux
we also update ScalarConfigPath(). We make no functional change
to the paths returned on macOS; however, errors due to an
undefined $HOME variable or other exceptions are now always
reported. On Linux, we implement the user-specific paths from
the XDG specification.
Note that on Linux we do not yet create missing directories with
a 0700 mode, as specified by the XDG standard, and we also do not
check system-wide locations from either the $XDG_{CONFIG,DATA}_DIRS
environment variables or the defaults for them. The latter
might require more extensive refactoring to distinguish cases
in which user-specific data or configurations are to be written,
as these should possibly not be written to system-wide files.
We update the CloneWithDefaultLocalCacheLocation() functional test so it accommodates variable local cache paths on Linux, depending on whether $XDG_CACHE_HOME is set or not. This allows the test to succeed on developer systems where we want to avoid polluting the default ~/.cache/scalar directory during test runs and so set $XDG_CACHE_HOME explicitly. Note that the new ScalarTestConfig.DefaultLocalCacheRoot accessor returns a path on Windows which should be "C:\.scalarCache", to match what would be supplied by the TryGetDefaultLocalCacheRoot() method of the WindowsPlatform if it were passed the default functional test enlistment root path. However, this Windows- specific DefaultLocalCacheRoot value is unused, as the only functional test which uses it is CloneWithDefaultLocalCacheLocation() and that test belongs to the POSIXOnly category.
chrisd8088
force-pushed
the
linux-initial-func-tests
branch
from
94ab823 to
a4cee25
Compare
chrisd8088
changed the title
chrisd8088
changed the title
There was a problem hiding this comment.
I only have one nit-picky comment, but shouldn't we just review and merge #431 since it actually runs the functional tests?
|
I broke this off from #431 because I didn't want to have too many notes in each PR's description; they're fairly long as it is independently. And this one was already getting a bit long in the tooth, what with the XDG work. Anyway, that was my thinking on that ... mostly so the merge commit notes don't have to be too long. |
Per PR advice from derrickstolee, we move all the fixed path and environment variable name strings used for local cache, config, and data path construction into ScalarConstants and TestConstants.
derrickstolee
approved these changes
Sep 14, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds functional test scripts for Linux, adjusts a number of Mac-specific syscall wrappers and
FileSystemRunnersto be either POSIX-generic or have Linux-specific equivalents, makes some minor updates to align Linux packaging details with other platforms, and refactors the setup of the cache, config, and data directories on POSIX platforms to support (in the Linux case only) the XDG Base Directory Specification.It should be possible to review either commit-by-commit or as a single set of changes; the former might be slightly easier, though.
The (partial) support for the XDG specification comprises the majority of the changes in this PR, and is motivated by the fact that the functional tests depend on the value returned by
GetDataRootForScalarImplementation()inLinuxPlatform.Shared.cs, and we currently return the value of either a custom environment variable or a default of/var/run/scalar, which may not be accessible to a non-root user and thus cause the tests to fail. While we could return a simple$HOME/.scalarresponse, following the XDG specification allows us to make parallel changes so that on Linux we utilize the relevant locations for all three types of files, which by default will be:~/.cache/scalar~/.config/scalar~/.local/share/scalarAlthough our implementation is not 100% complete (yet), it does permit runs of the functional test suite on Linux, and moreover allows for runs which avoid polluting these locations by defining the relevant
$XDG_{CACHE,CONFIG,DATA}_HOMEenvironment variables.What remains to be done, if desired, is described in a comment in
LinuxPlatform.csas well as the description for commit 9e859df:We also slightly refactor the
CloneTests.CloneWithDefaultLocalCacheLocation()functional test, which runs on POSIX platforms only, so as to ensure it passes on Linux regardless of whether the$XDG_CACHE_HOMEenvironment variable is set or not. This change depends on the introduction of aScalarTestConfig.DefaultLocalCacheRootaccessor which returns the appropriate cache path on each platform.