f00dce842565498ea6cdb69dd316f06350577f66
JIRA: RHELCMP-4378
Signed-off-by: Ozan Unsal ounsal@redhat.com
This default doesn't seem to be helpful. It install RPM it will point to /usr/bin. I think either current working directory or no default at all would be better.
/usr/bin
The sort considers 1.2.0 to be higher than 1.10.0. It's not a common case, but can happen :)
1.2.0
1.10.0
It works as expected. Few more comments:
Would it be possible to generalize to any number of version components? Now it only works with versions X.Y.Z. With a different number, strange things happen.
Can you move the code into a module (either a new one or symlink.py) so that tests can be written for it?
Done
It works as expected. Few more comments: Would it be possible to generalize to any number of version components? Now it only works with versions X.Y.Z. With a different number, strange things happen. Can you move the code into a module (either a new one or symlink.py) so that tests can be written for it?
rebased onto fdbc9f91549fcff9bb690a4f83eacb2aefac0291
rebased onto 45baa3c830557cb62c48d028edc5d1d647c58483
rebased onto 7d1b095a2cb0e15788777fb34c6b9aedb6399f14
'(\d+)' should either be a raw string or the backslash should be escaped.
'(\d+)'
Unlike the previous version, this will have all symlinks pointing to the full one (even latest-X-1 will point to latest-X-1.0.0 rather than latest-X-1.0). I actually like that, it means one less step in getting to the actual data.
latest-X-1
latest-X-1.0.0
latest-X-1.0
I'd like to see a few more test cases though. Something like this:
diff --git a/tests/test_symlink.py b/tests/test_symlink.py index 832a997..b3227d5 100644 --- a/tests/test_symlink.py +++ b/tests/test_symlink.py @@ -124,9 +124,43 @@ class TestLatestSymlink(unittest.TestCase): symlink.print_latest_link(path, version_type=symlink.VersionType.MINOR) self.assertEqual(mock_out.getvalue(), 'latest-DP-1.0\n') - def test_update_symlinks(self): - path = self._write_compose() - symlink.create_latest_link(path, version_type=symlink.VersionType.FULL) - path = os.path.join(path, "..") - symlink.update_symlinks(path) - self.assertTrue(os.path.islink(os.path.join(path, 'latest-DP-1'))) + +class TestUpdateSymlinks(unittest.TestCase): + def setUp(self): + self.compose_dir = tempfile.mkdtemp() + + def tearDown(self): + shutil.rmtree(self.compose_dir) + + def create_full_symlink(self, compose_id): + os.symlink('not-needed', os.path.join(self.compose_dir, compose_id)) + + def assert_symlink(self, name, dest): + link = os.path.join(self.compose_dir, name) + self.assertTrue(os.path.islink(link)) + self.assertEqual(os.readlink(link), dest) + + def test_two_component(self): + self.create_full_symlink('latest-DP-1.0') + + symlink.update_symlinks(self.compose_dir) + + self.assert_symlink('latest-DP-1', 'latest-DP-1.0') + + def test_three_component(self): + self.create_full_symlink('latest-DP-1.0.0') + + symlink.update_symlinks(self.compose_dir) + + self.assert_symlink('latest-DP-1', 'latest-DP-1.0.0') + self.assert_symlink('latest-DP-1.0', 'latest-DP-1.0.0') + + def test_scenario(self): + for y in range(1, 4): + self.create_full_symlink('latest-DP-1.%d.0' % y) + symlink.update_symlinks(self.compose_dir) + + self.assert_symlink('latest-DP-1', 'latest-DP-1.3.0') + self.assert_symlink('latest-DP-1.1', 'latest-DP-1.1.0') + self.assert_symlink('latest-DP-1.2', 'latest-DP-1.2.0') + self.assert_symlink('latest-DP-1.3', 'latest-DP-1.3.0')
A few more boring details: in order for the file to be installed, it needs to be listed in setup.py. Can you also create a man page for it? Copy an existing one, tweak as needed, preview with man ./docs/compose-update-latest-symlinks.1.
man ./docs/compose-update-latest-symlinks.1
The script crashes when path is not provided. Either make it a required argument, or use . as default.
.
rebased onto 11e26f89073560f1c7e8c381d63bd2eec0d7d268
rebased onto f00dce842565498ea6cdb69dd316f06350577f66
Looks good to me. Merging :tada:
Pull-Request has been merged by lsedlar
JIRA: RHELCMP-4378
Signed-off-by: Ozan Unsal ounsal@redhat.com