Archive tests #2229
Archive tests #2229
Conversation
c9b4e8d to
63b12ec
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, hanwen-flow The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
thanks! the 'tide' check says that it still needs the 'lgtm' label. |
|
LGTM, thanks. The package-scope "tmp" appears to be unused after these changes, might as well remove it. |
there are a couple of uses under TestUntarPathWithInvalidDest and below. It's for Windows, and I don't have a windows machine to do tests. |
A spot-check with |
Signed-off-by: Han-Wen Nienhuys <hanwen@engflow.com>
nothing. I was confused. Sorry! |
Before, the tests hardcoded filenames in a hardcoded /tmp. This
prevents parallelization, and is a security issue on multi-user
systems.
* use t.TempDir() where possible. I left the test that does something
with Windows alone.
* use &bytes.Buffer{} as dummy io.Writer
* use os.WriteFile() to simplify createEmptyFile
* drop unused tmp global variable
Signed-off-by: Han-Wen Nienhuys <hanwen@engflow.com>
An apparent cut & paste error. Before, the test was checking Xz another time. Signed-off-by: Han-Wen Nienhuys <hanwen@engflow.com>
63b12ec to
d91f63c
Compare
|
/lgtm |
@nalind - shouldn't this repo have some CI setup as well? It's vendored into important other packages, but the vendoring drops the tests. as an aside, IIUC, the tests assume they run as root. Would it be useful to add some t.Skip() or t.Fail() statements to the effect? |
|
Ideally, CI would cover more things, yes. Tests that we know will fail without certain privileges could stand to skip themselves if they know they don't have them, and at that point, having CI ensure that they run both with and without those privileges would be helpful. |
Some random cleanups I spotted when reading over the code.