Conversation
|
It probably makes sense to add a test. |
|
Done. :) @scheibelp Maybe you can look at this since you are already familiar with the original code. |
lib/spack/spack/test/cmd/view.py
Outdated
| install('libdwarf') | ||
| for symlink in [True, False]: | ||
| cmd = ["hardlink", "symlink"][symlink] | ||
| viewpath = str(tmpdir.mkdir('view_{}'.format(cmd))) |
There was a problem hiding this comment.
The {} format syntax doesn't work in python 2.6, indices are required
lib/spack/spack/test/cmd/view.py
Outdated
| tmpdir, builtin_mock, mock_archive, mock_fetch, config, | ||
| install_mockery): | ||
| install('libdwarf') | ||
| for symlink in [True, False]: |
There was a problem hiding this comment.
IMO this (and indexing by boolean on the next line) is a bit esoteric: I'd prefer for symlink, cmd in [(True, 'symlink'), (False, 'hardlink')]
There was a problem hiding this comment.
I think this is a good place to use parametrized tests. Grep around for pytest.mark.parametrized in the other tests and you can probably find an example -- you can let pytest do this work for you and just refer to parameters.
kljohann
force-pushed
the
hardlink
branch
2 times, most recently
from
3211bd6 to
6b453f5
Compare
|
Changed to use parametrized tests. |
lib/spack/spack/test/cmd/view.py
Outdated
| tmpdir, builtin_mock, mock_archive, mock_fetch, config, | ||
| install_mockery, cmd): | ||
| install('libdwarf') | ||
| viewpath = str(tmpdir.mkdir('view_{}'.format(cmd))) |
There was a problem hiding this comment.
If you replace view_{} with view{0}view_{0} all test failures should be resolved.
|
Sorry, I forgot about the python 2.6 issue. Fixed and thanks for your help :) |
Totally fine - thanks for fixing the bug! |
3 participants
This fixes a "typo" in
spack view hardlinkas introduced in #3227 / #5415. (os.hardlinkdoes not exist)