Nine tests fail on an ordinary Windows machine, eight in backend/local and
TestEnvironmentVariables in cmdtest, all with
symlink file.txt \?\C:\Users\...\symlink.txt: A required privilege is not
held by the client.
Windows grants SeCreateSymbolicLinkPrivilege only to an elevated process or one
running with Developer Mode enabled, and a default install gives an ordinary
user neither. CI does not see this because the windows-latest runner is
elevated, so the failures only show up on a contributor's own machine, where
AGENTS.md asks for make quicktest to pass before opening a pull request.
cmdtest already recognised the situation and attached a note to the failure
saying the test could safely be ignored. If it is safe to ignore then the test
knows it cannot run, so skip it and say why instead.
backend/local gains a helper that tries a symlink in t.TempDir() and skips if it
cannot make one, called from the six tests that need the privilege. Where a
platform can create symlinks the probe succeeds and nothing is skipped, so other
platforms are unchanged.
TestMetadata is skipped whole because it creates its symlink before anything
else and the object built from it is used throughout.
TestSymlinkEscapeConcurrent is left alone: it goes through putLink and ignores
the error, so it never needed the privilege.
This removes logrus which is not developed any more and replaces it
with the new log/slog from the Go standard library.
It implements its own slog Handler which is backwards compatible with
all of rclone's previous logging modes.
After the config re-organisation, the setting of stringArray config
values (eg `--exclude` set with `RCLONE_EXCLUDE`) was broken and gave
a message like this for `RCLONE_EXCLUDE=*.jpg`:
Failed to load "filter" default values: failed to initialise "filter" options:
couldn't parse config item "exclude" = "*.jpg" as []string: parsing "*.jpg" as []string failed:
invalid character '/' looking for beginning of value
This was caused by the parser trying to parse the input string as a
JSON value.
When the config was re-organised it was thought that the internal
representation of stringArray values was not important as it was never
visible externally, however this turned out not to be true.
A defined representation was chosen - a comma separated string and
this was documented and tests were introduced in this patch.
This potentially introduces a very small backwards incompatibility. In
rclone v1.67.0
RCLONE_EXCLUDE=a,b
Would be interpreted as
--exclude "a,b"
Whereas this new code will interpret it as
--exclude "a" --exclude "b"
The benefit of being able to set multiple values with an environment
variable was deemed to outweigh the very small backwards compatibility
risk.
If a value with a `,` is needed, then use CSV escaping, eg
RCLONE_EXCLUDE="a,b"
(Note this needs to have the quotes in so at the unix shell that would be
RCLONE_EXCLUDE='"a,b"'
Fixes#8063
This changes log statements from log to fs package, which is required for --use-json-log
to properly make log output in JSON format. The recently added custom linting rule,
handled by ruleguard via gocritic via golangci-lint, warns about these and suggests
the alternative. Fixing was therefore basically running "golangci-lint run --fix",
although some manual fixup of mainly imports are necessary following that.
There were a lot of instances of this lint error
printf: non-constant format string in call to github.com/rclone/rclone/fs.Logf (govet)
Which were fixed by re-arranging the arguments and adding "%s".
There were quite a few genuine bugs which were found too.
There was no easy way to automatically test the end-to-end functionality
of commands, flags, environment variables etc.
The need for end-to-end testing was highlighted by the issues fixed
in #5341. There was no automated test to continually verify current
behaviour, nor a framework to quickly test the correctness of the fixes.
This change adds an end-to-end testing framework in the cmdtest folder.
It has some simple examples in func TestCmdTest in cmdtest_test.go. The
tests should be readable by anybody familiar with rclone and look like
this:
// Test the rclone version command with debug logging (-vv)
out, err = rclone("version", "-vv")
if assert.NoError(t, err) {
assert.Contains(t, out, "rclone v")
assert.Contains(t, out, "os/version:")
assert.Contains(t, out, " DEBUG : ")
}
The end-to-end tests are executed just like the Go unit tests, that is:
go test ./cmdtest -v
The change also contains a thorough test of environment variables in
environment_test.go.
Thanks to @ncw for encouragement and introduction to the TestMain trick.
DhevenddraandNick Craig-Wood
shenpengfengandGitHub
albertony
Dimitri PapadopoulosandNick Craig-Wood
Eng Zer JunandNick Craig-Wood
Josh Sorefandalbertony
Ole FrostandNick Craig-Wood