Move Changeset, Node and Repository classes to model.py

[Ryan J Ollos]

As discussed in #11609, move theChangeset, Node and Repository classes to a new model.py module in trac/versioncontrol.

[Ryan J Ollos]

Looking it at it more closely now, it could be that the reason the classes are in api.py rather than model.py is that they are abstract classes.

[Ryan J Ollos]

Some additional experimental changes in log:rjollos.git:t11610

The classes in the abc module were utilized in #11581. There are other classes in the repository which are made abstract by raising NotImplementedErrors in methods, but the ABCMeta class forces subclasses to adhere to the contract (comment:2:ticket:11581), and more clearly identifies the base classes as abstract.

Some unit tests are currently failing due to:

1. GitRepository doesn't implemented get_path_history (#9775):

   ======================================================================
   ERROR: test_node_get_history_with_empty_commit (tracopt.versioncontrol.git.tests.PyGIT.NormalTestCase)
   ----------------------------------------------------------------------
   Traceback (most recent call last):
     File "/home/user/Workspace/tracdev/teo-rjollos.git/tracopt/versioncontrol/git/tests/PyGIT.py", line 251, in test_node_get_history_with_empty_commit
       repos = self.env.get_repository('gitrepos')
     File "/home/user/Workspace/tracdev/teo-rjollos.git/trac/env.py", line 567, in get_repository
       return RepositoryManager(self).get_repository(reponame)
     File "/home/user/Workspace/tracdev/teo-rjollos.git/trac/versioncontrol/api.py", line 575, in get_repository
       repoinfo.copy())
     File "/home/user/Workspace/tracdev/teo-rjollos.git/tracopt/versioncontrol/git/git_fs.py", line 280, in get_repository
       use_committer_time=self.use_committer_time,
   TypeError: Can't instantiate abstract class GitRepository with abstract methods get_path_history
   

2. A Mock object can't be created directly from a base class with abstractmethods. It is possible that we need to modify Mock in order to deal with abstract base classes. I'm not sure yet what is the best approach to take on this.

   ======================================================================
   ERROR: test_clean_sync (trac.versioncontrol.tests.cache.CacheTestCase)
   ----------------------------------------------------------------------
   Traceback (most recent call last):
     File "/home/user/Workspace/tracdev/teo-rjollos.git/trac/versioncontrol/tests/cache.py", line 158, in test_clean_sync
       youngest_rev=2)
     File "/home/user/Workspace/tracdev/teo-rjollos.git/trac/versioncontrol/tests/cache.py", line 54, in get_repos
       next_rev=(lambda x: int(x) < youngest_rev and x + 1 \
     File "/home/user/Workspace/tracdev/teo-rjollos.git/trac/test.py", line 94, in Mock
       mock = cls(*initargs)
   TypeError: Can't instantiate abstract class Mock with abstract methods close, get_changes, get_changeset, get_node, get_oldest_rev, get_path_history, get_youngest_rev, next_rev, normalize_path, normalize_rev, previous_rev, rev_older_than
   

[Ryan J Ollos]

Changeset is instantiated directly in CachedRepository​. I fixed the failing test by instead instantiating the derived class. The only place I see the object used is: tags/trac-1.0.1/trac/versioncontrol/api.py@:662,674#L657​. So now a GitChangeset, SubversionChangeset or other derived type is passed to the change listener. Are there any possible negative consequences to this that I'm not seeing?

As for creating Mock instances of ABCs, it seemed most straightforward to just remove the abstract methods from the class. Like any other methods in a mocked object, an AttributeError will be raised if methods omitted from the Mock are utilized in a function under test.

Latest changes added to log:rjollos.git:t11610.

[Jun Omae]

Changeset is instantiated directly in CachedRepository​. I fixed the failing test by instead instantiating the derived class. The only place I see the object used is: tags/trac-1.0.1/trac/versioncontrol/api.py@:662,674#L657​. So now a GitChangeset, SubversionChangeset or other derived type is passed to the change listener. Are there any possible negative consequences to this that I'm not seeing?

I think it would be good to create CachedChangeset instance rather than Changeset. Also, sync_changeset method is for only cached repository. Then, GitChangeset and SubversionChangeset never be passed to the change listener.

See [058936ed/jomae.git].

As for creating Mock instances of ABCs, it seemed most straightforward to just remove the abstract methods from the class. Like any other methods in a mocked object, an AttributeError will be raised if methods omitted from the Mock are utilized in a function under test.

We could add support for ABCMeta class as base class of Mock. See [8f55ca19/jomae.git].

[Ryan J Ollos]

Those changes look good, thanks. Committed to trunk in [12782:12783].

We'll need to implement get_path_history in the MercurialPlugin's Repository subclass. I don't have the necessary permission to push to the branch.

tracext/hg/backend.py

@@ -33 +33 @@
 from trac.util import arity
 from trac.util.datefmt import FixedOffset, utc
 from trac.util.text import exception_to_unicode, shorten_line, to_unicode
-from trac.util.translation import domain_functions
+from trac.util.translation import _, domain_functions
 from trac.versioncontrol.api import Changeset, Node, Repository, \
                                     IRepositoryConnector, RepositoryManager, \
                                     NoSuchChangeset, NoSuchNode
@@ -747 +747 @@
     def get_youngest_rev(self):
         return self.changectx().hex()
 
+    def get_path_history(self, path, rev=None, limit=None):
+        raise TracError(_("Unsupported \"Show only adds and deletes\""))
+
     def previous_rev(self, rev, path=''): # FIXME: path ignored for now
         for p in self.changectx(rev).parents():
             if p: