manifest_xml: use Superproject to hold XML contentv2.23

Always create Superproject when there is a <superproject> tag, and have
it hold the XML content, similar to how other manifest elements are
handled.

This also adds SetQuiet and SetPrintMessages to Superproject
consistent with manifest.SetUseLocalManifests.

Change-Id: I522bf3da542006575799f0640c67f7052704f266
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/334641
Reviewed-by: Mike Frysinger <vapier@google.com>
Reviewed-by: Raman Tenneti <rtenneti@google.com>
Tested-by: LaMont Jones <lamontjones@google.com>

7 files changed, 125 insertions, 117 deletions

diff --git a/git_superproject.py b/git_superproject.py
index f4dbb27b..9f585e5d 100644
--- a/git_superproject.py
+++ b/git_superproject.py
@@ -71,42 +71,50 @@ class Superproject(object):
   lookup of commit ids for all projects. It contains _project_commit_ids which
   is a dictionary with project/commit id entries.
   """
-  def __init__(self, manifest, repodir, git_event_log,
-               superproject_dir='exp-superproject', quiet=False, print_messages=False):
+  def __init__(self, manifest, name, remote, revision,
+               superproject_dir='exp-superproject'):
     """Initializes superproject.
 
     Args:
       manifest: A Manifest object that is to be written to a file.
-      repodir: Path to the .repo/ dir for holding all internal checkout state.
-          It must be in the top directory of the repo client checkout.
-      git_event_log: A git trace2 event log to log events.
-      superproject_dir: Relative path under |repodir| to checkout superproject.
-      quiet:  If True then only print the progress messages.
-      print_messages: if True then print error/warning messages.
+      name: The unique name of the superproject
+      remote: The RemoteSpec for the remote.
+      revision: The name of the git branch to track.
+      superproject_dir: Relative path under |manifest.subdir| to checkout
+          superproject.
     """
     self._project_commit_ids = None
     self._manifest = manifest
-    self._git_event_log = git_event_log
-    self._quiet = quiet
-    self._print_messages = print_messages
-    self._branch = manifest.branch
-    self._repodir = os.path.abspath(repodir)
+    self.name = name
+    self.remote = remote
+    self.revision = self._branch = revision
+    self._repodir = manifest.repodir
     self._superproject_dir = superproject_dir
     self._superproject_path = manifest.SubmanifestInfoDir(manifest.path_prefix,
                                                           superproject_dir)
     self._manifest_path = os.path.join(self._superproject_path,
                                        _SUPERPROJECT_MANIFEST_NAME)
-    git_name = ''
-    if self._manifest.superproject:
-      remote = self._manifest.superproject['remote']
-      git_name = hashlib.md5(remote.name.encode('utf8')).hexdigest() + '-'
-      self._branch = self._manifest.superproject['revision']
-      self._remote_url = remote.url
-    else:
-      self._remote_url = None
+    git_name = hashlib.md5(remote.name.encode('utf8')).hexdigest() + '-'
+    self._remote_url = remote.url
     self._work_git_name = git_name + _SUPERPROJECT_GIT_NAME
     self._work_git = os.path.join(self._superproject_path, self._work_git_name)
 
+    # The following are command arguemnts, rather then superproject attributes,
+    # and where included here originally.  They should eventually become
+    # arguments that are passed down from the public methods, instead of being
+    # treated as attributes.
+    self._git_event_log = None
+    self._quiet = False
+    self._print_messages = False
+
+  def SetQuiet(self, value):
+    """Set the _quiet attribute."""
+    self._quiet = value
+
+  def SetPrintMessages(self, value):
+    """Set the _print_messages attribute."""
+    self._print_messages = value
+
   @property
   def project_commit_ids(self):
     """Returns a dictionary of projects and their commit ids."""
@@ -215,12 +223,16 @@ class Superproject(object):
                        f'return code: {retval}, stderr: {p.stderr}')
     return data
 
-  def Sync(self):
+  def Sync(self, git_event_log):
     """Gets a local copy of a superproject for the manifest.
 
+    Args:
+      git_event_log: an EventLog, for git tracing.
+
     Returns:
       SyncResult
     """
+    self._git_event_log = git_event_log
     if not self._manifest.superproject:
       self._LogWarning(f'superproject tag is not defined in manifest: '
                        f'{self._manifest.manifestFile}')
@@ -248,7 +260,7 @@ class Superproject(object):
     Returns:
       CommitIdsResult
     """
-    sync_result = self.Sync()
+    sync_result = self.Sync(self._git_event_log)
     if not sync_result.success:
       return CommitIdsResult(None, sync_result.fatal)
 
@@ -313,7 +325,7 @@ class Superproject(object):
     # Skip the project if it comes from the local manifest.
     return project.manifest.IsFromLocalManifest(project)
 
-  def UpdateProjectsRevisionId(self, projects):
+  def UpdateProjectsRevisionId(self, projects, git_event_log):
     """Update revisionId of every project in projects with the commit id.
 
     Args:
@@ -322,6 +334,7 @@ class Superproject(object):
     Returns:
       UpdateProjectsResult
     """
+    self._git_event_log = git_event_log
     commit_ids_result = self._GetAllProjectsCommitIds()
     commit_ids = commit_ids_result.commit_ids
     if not commit_ids:
@@ -397,7 +410,7 @@ def _UseSuperprojectFromConfiguration():
 
 def PrintMessages(opt, manifest):
   """Returns a boolean if error/warning messages are to be printed."""
-  return opt.use_superproject is not None or manifest.superproject
+  return opt.use_superproject is not None or bool(manifest.superproject)
 
 
 def UseSuperproject(opt, manifest):
@@ -409,7 +422,7 @@ def UseSuperproject(opt, manifest):
     client_value = manifest.manifestProject.use_superproject
     if client_value is not None:
       return client_value
-    else:
-      if not manifest.superproject:
-        return False
+    elif manifest.superproject:
       return _UseSuperprojectFromConfiguration()
+    else:
+      return False
diff --git a/manifest_xml.py b/manifest_xml.py
index 8718dc54..7d19d63e 100644
--- a/manifest_xml.py
+++ b/manifest_xml.py
@@ -24,6 +24,7 @@ import urllib.parse
 import gitc_utils
 from git_config import GitConfig, IsId
 from git_refs import R_HEADS, HEAD
+from git_superproject import Superproject
 import platform_utils
 from project import (Annotation, RemoteSpec, Project, RepoProject,
                      ManifestProject)
@@ -670,17 +671,17 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     if self._superproject:
       root.appendChild(doc.createTextNode(''))
       e = doc.createElement('superproject')
-      e.setAttribute('name', self._superproject['name'])
+      e.setAttribute('name', self._superproject.name)
       remoteName = None
       if d.remote:
         remoteName = d.remote.name
-      remote = self._superproject.get('remote')
+      remote = self._superproject.remote
       if not d.remote or remote.orig_name != remoteName:
         remoteName = remote.orig_name
         e.setAttribute('remote', remoteName)
       revision = remote.revision or d.revisionExpr
-      if not revision or revision != self._superproject['revision']:
-        e.setAttribute('revision', self._superproject['revision'])
+      if not revision or revision != self._superproject.revision:
+        e.setAttribute('revision', self._superproject.revision)
       root.appendChild(e)
 
     if self._contactinfo.bugurl != Wrapper().BUG_URL:
@@ -984,7 +985,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     self._default = None
     self._submanifests = {}
     self._repo_hooks_project = None
-    self._superproject = {}
+    self._superproject = None
     self._contactinfo = ContactInfo(Wrapper().BUG_URL)
     self._notice = None
     self.branch = None
@@ -1052,20 +1053,19 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
 
       # Now that we have loaded this manifest, load any submanifest  manifests
       # as well.  We need to do this after self._loaded is set to avoid looping.
-      if self._outer_client:
-        for name in self._submanifests:
-          tree = self._submanifests[name]
-          spec = tree.ToSubmanifestSpec(self)
-          present = os.path.exists(os.path.join(self.subdir, MANIFEST_FILE_NAME))
-          if present and tree.present and not tree.repo_client:
-            if initial_client and initial_client.topdir == self.topdir:
-              tree.repo_client = self
-              tree.present = present
-            elif not os.path.exists(self.subdir):
-              tree.present = False
-          if present and tree.present:
-            tree.repo_client._Load(initial_client=initial_client,
-                                   submanifest_depth=submanifest_depth + 1)
+      for name in self._submanifests:
+        tree = self._submanifests[name]
+        spec = tree.ToSubmanifestSpec(self)
+        present = os.path.exists(os.path.join(self.subdir, MANIFEST_FILE_NAME))
+        if present and tree.present and not tree.repo_client:
+          if initial_client and initial_client.topdir == self.topdir:
+            tree.repo_client = self
+            tree.present = present
+          elif not os.path.exists(self.subdir):
+            tree.present = False
+        if present and tree.present:
+          tree.repo_client._Load(initial_client=initial_client,
+                                 submanifest_depth=submanifest_depth + 1)
 
   def _ParseManifestXml(self, path, include_root, parent_groups='',
                         restrict_includes=True):
@@ -1267,11 +1267,10 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
       if node.nodeName == 'superproject':
         name = self._reqatt(node, 'name')
         # There can only be one superproject.
-        if self._superproject.get('name'):
+        if self._superproject:
           raise ManifestParseError(
               'duplicate superproject in %s' %
               (self.manifestFile))
-        self._superproject['name'] = name
         remote_name = node.getAttribute('remote')
         if not remote_name:
           remote = self._default.remote
@@ -1280,14 +1279,16 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         if remote is None:
           raise ManifestParseError("no remote for superproject %s within %s" %
                                    (name, self.manifestFile))
-        self._superproject['remote'] = remote.ToRemoteSpec(name)
         revision = node.getAttribute('revision') or remote.revision
         if not revision:
           revision = self._default.revisionExpr
         if not revision:
           raise ManifestParseError('no revision for superproject %s within %s' %
                                    (name, self.manifestFile))
-        self._superproject['revision'] = revision
+        self._superproject = Superproject(self,
+                                          name=name,
+                                          remote=remote.ToRemoteSpec(name),
+                                          revision=revision)
       if node.nodeName == 'contactinfo':
         bugurl = self._reqatt(node, 'bugurl')
         # This element can be repeated, later entries will clobber earlier ones.
diff --git a/project.py b/project.py
index b8d834aa..5ed103b9 100644
--- a/project.py
+++ b/project.py
@@ -36,7 +36,6 @@ from git_trace2_event_log import EventLog
 from error import GitError, UploadError, DownloadError
 from error import ManifestInvalidRevisionError, ManifestInvalidPathError
 from error import NoManifestException, ManifestParseError
-from git_superproject import Superproject
 import platform_utils
 import progress
 from repo_trace import IsTrace, Trace
diff --git a/subcmds/init.py b/subcmds/init.py
index 65b63efd..99f30dce 100644
--- a/subcmds/init.py
+++ b/subcmds/init.py
@@ -25,7 +25,6 @@ from project import SyncBuffer
 from git_config import GitConfig
 from git_command import git_require, MIN_GIT_VERSION_SOFT, MIN_GIT_VERSION_HARD
 import fetch
-import git_superproject
 import platform_utils
 from wrapper import Wrapper
 
diff --git a/subcmds/sync.py b/subcmds/sync.py
index 9e783205..4d0a5ec6 100644
--- a/subcmds/sync.py
+++ b/subcmds/sync.py
@@ -304,12 +304,10 @@ later is required to fix a server side protocol bug.
     Returns:
       Returns path to the overriding manifest file instead of None.
     """
+    superproject = self.manifest.superproject
+    superproject.SetQuiet(opt.quiet)
     print_messages = git_superproject.PrintMessages(opt, self.manifest)
-    superproject = git_superproject.Superproject(self.manifest,
-                                                 self.repodir,
-                                                 self.git_event_log,
-                                                 quiet=opt.quiet,
-                                                 print_messages=print_messages)
+    superproject.SetPrintMessages(print_messages)
     if opt.local_only:
       manifest_path = superproject.manifest_path
       if manifest_path:
@@ -319,7 +317,8 @@ later is required to fix a server side protocol bug.
     all_projects = self.GetProjects(args,
                                     missing_ok=True,
                                     submodules_ok=opt.fetch_submodules)
-    update_result = superproject.UpdateProjectsRevisionId(all_projects)
+    update_result = superproject.UpdateProjectsRevisionId(
+        all_projects, git_event_log=self.git_event_log)
     manifest_path = update_result.manifest_path
     superproject_logging_data['updatedrevisionid'] = bool(manifest_path)
     if manifest_path:
diff --git a/tests/test_git_superproject.py b/tests/test_git_superproject.py
index a24fc7f0..1e7b1201 100644
--- a/tests/test_git_superproject.py
+++ b/tests/test_git_superproject.py
@@ -68,8 +68,10 @@ class SuperprojectTestCase(unittest.TestCase):
   <project path="art" name="platform/art" groups="notdefault,platform-""" + self.platform + """
   " /></manifest>
 """)
-    self._superproject = git_superproject.Superproject(manifest, self.repodir,
-                                                       self.git_event_log)
+    self._superproject = git_superproject.Superproject(
+        manifest, name='superproject',
+        remote=manifest.remotes.get('default-remote').ToRemoteSpec('superproject'),
+        revision='refs/heads/main')
 
   def tearDown(self):
     """Tear down superproject every time."""
@@ -125,12 +127,7 @@ class SuperprojectTestCase(unittest.TestCase):
 <manifest>
 </manifest>
 """)
-    superproject = git_superproject.Superproject(manifest, self.repodir, self.git_event_log)
-    # Test that exit condition is false when there is no superproject tag.
-    sync_result = superproject.Sync()
-    self.assertFalse(sync_result.success)
-    self.assertFalse(sync_result.fatal)
-    self.verifyErrorEvent()
+    self.assertIsNone(manifest.superproject)
 
   def test_superproject_get_superproject_invalid_url(self):
     """Test with an invalid url."""
@@ -141,8 +138,11 @@ class SuperprojectTestCase(unittest.TestCase):
   <superproject name="superproject"/>
 </manifest>
 """)
-    superproject = git_superproject.Superproject(manifest, self.repodir, self.git_event_log)
-    sync_result = superproject.Sync()
+    superproject = git_superproject.Superproject(
+        manifest, name='superproject',
+        remote=manifest.remotes.get('test-remote').ToRemoteSpec('superproject'),
+        revision='refs/heads/main')
+    sync_result = superproject.Sync(self.git_event_log)
     self.assertFalse(sync_result.success)
     self.assertTrue(sync_result.fatal)
 
@@ -155,17 +155,19 @@ class SuperprojectTestCase(unittest.TestCase):
   <superproject name="superproject"/>
 </manifest>
 """)
-    self._superproject = git_superproject.Superproject(manifest, self.repodir,
-                                                       self.git_event_log)
+    self._superproject = git_superproject.Superproject(
+        manifest, name='superproject',
+        remote=manifest.remotes.get('test-remote').ToRemoteSpec('superproject'),
+        revision='refs/heads/main')
     with mock.patch.object(self._superproject, '_branch', 'junk'):
-      sync_result = self._superproject.Sync()
+      sync_result = self._superproject.Sync(self.git_event_log)
       self.assertFalse(sync_result.success)
       self.assertTrue(sync_result.fatal)
 
   def test_superproject_get_superproject_mock_init(self):
     """Test with _Init failing."""
     with mock.patch.object(self._superproject, '_Init', return_value=False):
-      sync_result = self._superproject.Sync()
+      sync_result = self._superproject.Sync(self.git_event_log)
       self.assertFalse(sync_result.success)
       self.assertTrue(sync_result.fatal)
 
@@ -174,7 +176,7 @@ class SuperprojectTestCase(unittest.TestCase):
     with mock.patch.object(self._superproject, '_Init', return_value=True):
       os.mkdir(self._superproject._superproject_path)
       with mock.patch.object(self._superproject, '_Fetch', return_value=False):
-        sync_result = self._superproject.Sync()
+        sync_result = self._superproject.Sync(self.git_event_log)
         self.assertFalse(sync_result.success)
         self.assertTrue(sync_result.fatal)
 
@@ -230,7 +232,7 @@ class SuperprojectTestCase(unittest.TestCase):
                                return_value=data):
           # Create temporary directory so that it can write the file.
           os.mkdir(self._superproject._superproject_path)
-          update_result = self._superproject.UpdateProjectsRevisionId(projects)
+          update_result = self._superproject.UpdateProjectsRevisionId(projects, self.git_event_log)
           self.assertIsNotNone(update_result.manifest_path)
           self.assertFalse(update_result.fatal)
           with open(update_result.manifest_path, 'r') as fp:
@@ -256,22 +258,13 @@ class SuperprojectTestCase(unittest.TestCase):
 </manifest>
 """)
     self.maxDiff = None
-    self._superproject = git_superproject.Superproject(manifest, self.repodir,
-                                                       self.git_event_log)
-    self.assertEqual(len(self._superproject._manifest.projects), 1)
-    projects = self._superproject._manifest.projects
-    project = projects[0]
-    project.SetRevisionId('ABCDEF')
-    update_result = self._superproject.UpdateProjectsRevisionId(projects)
-    self.assertIsNone(update_result.manifest_path)
-    self.assertFalse(update_result.fatal)
-    self.verifyErrorEvent()
+    self.assertIsNone(manifest.superproject)
     self.assertEqual(
         sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'
         '<remote fetch="http://localhost" name="default-remote"/>'
         '<default remote="default-remote" revision="refs/heads/main"/>'
-        '<project name="test-name" revision="ABCDEF" upstream="refs/heads/main"/>'
+        '<project name="test-name"/>'
         '</manifest>')
 
   def test_superproject_update_project_revision_id_from_local_manifest_group(self):
@@ -290,8 +283,10 @@ class SuperprojectTestCase(unittest.TestCase):
   " /></manifest>
 """)
     self.maxDiff = None
-    self._superproject = git_superproject.Superproject(manifest, self.repodir,
-                                                       self.git_event_log)
+    self._superproject = git_superproject.Superproject(
+        manifest, name='superproject',
+        remote=manifest.remotes.get('default-remote').ToRemoteSpec('superproject'),
+        revision='refs/heads/main')
     self.assertEqual(len(self._superproject._manifest.projects), 2)
     projects = self._superproject._manifest.projects
     data = ('160000 commit 2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea\tart\x00')
@@ -302,7 +297,7 @@ class SuperprojectTestCase(unittest.TestCase):
                                return_value=data):
           # Create temporary directory so that it can write the file.
           os.mkdir(self._superproject._superproject_path)
-          update_result = self._superproject.UpdateProjectsRevisionId(projects)
+          update_result = self._superproject.UpdateProjectsRevisionId(projects, self.git_event_log)
           self.assertIsNotNone(update_result.manifest_path)
           self.assertFalse(update_result.fatal)
           with open(update_result.manifest_path, 'r') as fp:
@@ -337,8 +332,10 @@ class SuperprojectTestCase(unittest.TestCase):
   " /></manifest>
 """)
     self.maxDiff = None
-    self._superproject = git_superproject.Superproject(manifest, self.repodir,
-                                                       self.git_event_log)
+    self._superproject = git_superproject.Superproject(
+        manifest, name='superproject',
+        remote=manifest.remotes.get('default-remote').ToRemoteSpec('superproject'),
+        revision='refs/heads/main')
     self.assertEqual(len(self._superproject._manifest.projects), 3)
     projects = self._superproject._manifest.projects
     data = ('160000 commit 2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea\tart\x00'
@@ -350,7 +347,7 @@ class SuperprojectTestCase(unittest.TestCase):
                                return_value=data):
           # Create temporary directory so that it can write the file.
           os.mkdir(self._superproject._superproject_path)
-          update_result = self._superproject.UpdateProjectsRevisionId(projects)
+          update_result = self._superproject.UpdateProjectsRevisionId(projects, self.git_event_log)
           self.assertIsNotNone(update_result.manifest_path)
           self.assertFalse(update_result.fatal)
           with open(update_result.manifest_path, 'r') as fp:
diff --git a/tests/test_manifest_xml.py b/tests/test_manifest_xml.py
index cb3eb855..ede41547 100644
--- a/tests/test_manifest_xml.py
+++ b/tests/test_manifest_xml.py
@@ -289,8 +289,8 @@ class XmlManifestTests(ManifestParseTestCase):
   <x-custom-tag>X tags are always ignored</x-custom-tag>
 </manifest>
 """)
-    self.assertEqual(manifest.superproject['name'], 'superproject')
-    self.assertEqual(manifest.superproject['remote'].name, 'test-remote')
+    self.assertEqual(manifest.superproject.name, 'superproject')
+    self.assertEqual(manifest.superproject.remote.name, 'test-remote')
     self.assertEqual(
         sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'
@@ -569,10 +569,10 @@ class SuperProjectElementTests(ManifestParseTestCase):
   <superproject name="superproject"/>
 </manifest>
 """)
-    self.assertEqual(manifest.superproject['name'], 'superproject')
-    self.assertEqual(manifest.superproject['remote'].name, 'test-remote')
-    self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject')
-    self.assertEqual(manifest.superproject['revision'], 'refs/heads/main')
+    self.assertEqual(manifest.superproject.name, 'superproject')
+    self.assertEqual(manifest.superproject.remote.name, 'test-remote')
+    self.assertEqual(manifest.superproject.remote.url, 'http://localhost/superproject')
+    self.assertEqual(manifest.superproject.revision, 'refs/heads/main')
     self.assertEqual(
         sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'
@@ -591,10 +591,10 @@ class SuperProjectElementTests(ManifestParseTestCase):
   <superproject name="superproject" revision="refs/heads/stable" />
 </manifest>
 """)
-    self.assertEqual(manifest.superproject['name'], 'superproject')
-    self.assertEqual(manifest.superproject['remote'].name, 'test-remote')
-    self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject')
-    self.assertEqual(manifest.superproject['revision'], 'refs/heads/stable')
+    self.assertEqual(manifest.superproject.name, 'superproject')
+    self.assertEqual(manifest.superproject.remote.name, 'test-remote')
+    self.assertEqual(manifest.superproject.remote.url, 'http://localhost/superproject')
+    self.assertEqual(manifest.superproject.revision, 'refs/heads/stable')
     self.assertEqual(
         sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'
@@ -613,10 +613,10 @@ class SuperProjectElementTests(ManifestParseTestCase):
   <superproject name="superproject" revision="refs/heads/stable" />
 </manifest>
 """)
-    self.assertEqual(manifest.superproject['name'], 'superproject')
-    self.assertEqual(manifest.superproject['remote'].name, 'test-remote')
-    self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject')
-    self.assertEqual(manifest.superproject['revision'], 'refs/heads/stable')
+    self.assertEqual(manifest.superproject.name, 'superproject')
+    self.assertEqual(manifest.superproject.remote.name, 'test-remote')
+    self.assertEqual(manifest.superproject.remote.url, 'http://localhost/superproject')
+    self.assertEqual(manifest.superproject.revision, 'refs/heads/stable')
     self.assertEqual(
         sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'
@@ -635,10 +635,10 @@ class SuperProjectElementTests(ManifestParseTestCase):
   <superproject name="superproject" revision="refs/heads/stable" />
 </manifest>
 """)
-    self.assertEqual(manifest.superproject['name'], 'superproject')
-    self.assertEqual(manifest.superproject['remote'].name, 'test-remote')
-    self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject')
-    self.assertEqual(manifest.superproject['revision'], 'refs/heads/stable')
+    self.assertEqual(manifest.superproject.name, 'superproject')
+    self.assertEqual(manifest.superproject.remote.name, 'test-remote')
+    self.assertEqual(manifest.superproject.remote.url, 'http://localhost/superproject')
+    self.assertEqual(manifest.superproject.revision, 'refs/heads/stable')
     self.assertEqual(
         sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'
@@ -657,10 +657,10 @@ class SuperProjectElementTests(ManifestParseTestCase):
   <superproject name="platform/superproject" remote="superproject-remote"/>
 </manifest>
 """)
-    self.assertEqual(manifest.superproject['name'], 'platform/superproject')
-    self.assertEqual(manifest.superproject['remote'].name, 'superproject-remote')
-    self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/platform/superproject')
-    self.assertEqual(manifest.superproject['revision'], 'refs/heads/main')
+    self.assertEqual(manifest.superproject.name, 'platform/superproject')
+    self.assertEqual(manifest.superproject.remote.name, 'superproject-remote')
+    self.assertEqual(manifest.superproject.remote.url, 'http://localhost/platform/superproject')
+    self.assertEqual(manifest.superproject.revision, 'refs/heads/main')
     self.assertEqual(
         sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'
@@ -679,9 +679,9 @@ class SuperProjectElementTests(ManifestParseTestCase):
   <superproject name="superproject" remote="default-remote"/>
 </manifest>
 """)
-    self.assertEqual(manifest.superproject['name'], 'superproject')
-    self.assertEqual(manifest.superproject['remote'].name, 'default-remote')
-    self.assertEqual(manifest.superproject['revision'], 'refs/heads/main')
+    self.assertEqual(manifest.superproject.name, 'superproject')
+    self.assertEqual(manifest.superproject.remote.name, 'default-remote')
+    self.assertEqual(manifest.superproject.revision, 'refs/heads/main')
     self.assertEqual(
         sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'