I'll comment on code here because pagure isn't good with preserving inline comments when changes are pushed.

• What about not document in yml → document not in yml? It's easier, more natural to read.
• If you break lines, please take care that the continued line doesn't end up on the same indentation level as the following block so it's immediately clear what belongs to the block (yeah, I need to clean that up in existing code). E.g. this...:

  if ("license" in yml["data"] and isinstance(yml["data"]["license"], dict)
      and "content" in yml["data"]["license"]):
      self.content_licenses = set(yml["data"]["license"]["content"])
  

  ...should rather look like that:

  if ("license" in yml["data"] and isinstance(yml["data"]["license"], dict)
          and "content" in yml["data"]["license"]):
      self.content_licenses = set(yml["data"]["license"]["content"])
  

Hang on, not through the code yet -- slip of the finger (clicked on "Update Issue" rather than "Preview").

TestIO:

• test_yaml(): I find creating document only for the default value of yaml is superfluous and forces one to read back on the following if not yaml: block. Rather put this inside the block and get rid of the additional variable?
• FIx tense: "could not found expected ':'" → "could not find expected ':'"—may need fixing the place where the exception is raised, too.

• Don't put whitespace around = for keyword argument defaults/values (PEP8). E.g.:

  def test_object_value(self, yaml = None, value = ""):
  

  should be:

  def test_object_value(self, yaml=None, value=""):
  

• You can use a generator rather than a list comprehension if you use it only once, likewise forgo using str.find() if you only want to check for the existence of a substring here:

  yaml = "\n".join([n for n in yaml.split("\n") if n.find("$VALUE") == -1])
  

  →

  yaml = "\n".join(n for n in yaml.split("\n") if "$VALUE" not in n)
  

• Many methods create a ModuleMetadata object mmd without ever using it.

• test_{version,release,summary,description}() and others have several similar calls to test_object_value() at the end, I like the loop you do in e.g. test_dependencies() better.

I think I've addressed the issues you have found except the "could not found expected" error message. This message is generated by the Python yaml module and therefore cannot be fixed in our code.

Thanks!

I think I've addressed the issues you have found except the "could not found expected" error message. This message is generated by the Python yaml module and therefore cannot be fixed in our code.

Ugh. Perhaps add a comment there? Otherwise someone coming across it might want to "fix" it. You know what, make the regex r"could not f(?:ou|i)nd expected ':'" plus the comment, then we're prepared for if the issue is fixed in the yaml module.

I like throwing away the extra functionality in test_invalid_yaml which is never used anyway. :wink:

Besides that, rather than fixing stuff in a separate commit, please fix them in the commit where they're introduced (git rebase -i is your friend there, if not, poke me). We're grooming the PR, history is not yet sacrosanct. :smiley:

Merged in commit bf96a45a5b50125176199ba777fdcf2c2797c9bb.