Some unit tests break with upgrade to MyCapytain 2.0.8

[sonofmun]

Below I will list the broken tests with questions and suggestions for how to fix them.

[sonofmun]

https://github.com/Capitains/HookTest/blob/3c8e1b4bfc6ac62d9b41cdc9689d8d4d1d9b0b0b/tests/test_units.py#L551-L566

[sonofmun]

The tested text has no legal refsDecl and so it throws an exception in the new version of MyCapytain. That means this test is now testing refsDecl existence instead of counting words. I would suggest that we have a text that has a legal refsDecl that doesn't point to any actual textual nodes. This, I think will get around it.

[sonofmun]

https://github.com/Capitains/HookTest/blob/3c8e1b4bfc6ac62d9b41cdc9689d8d4d1d9b0b0b/tests/test_run.py#L102-L112

[PonteIneptique]

Agreed. I would recommend a smallish file though

[sonofmun]

This returns no status and thus the later lines that rely on the status of this test fail. I am not yet sure why this happens, but will look at it more closely.

[PonteIneptique]

• Addendum : we should make sure we catch the new exception because they might lead to crash of the tests ;)

[sonofmun]

https://github.com/Capitains/HookTest/blob/3c8e1b4bfc6ac62d9b41cdc9689d8d4d1d9b0b0b/tests/test_run.py#L348-L359

[sonofmun]

This has the same problem as the test_run_local_repo_errors test. Probably the same solution for that test will also work here.

[sonofmun]

Catching the error was the problem with the other two tests. Since a missing refsDecl now throws and error and stops the ingestion of a text as a CapitainsCtsText, the HookTest.capitains_units.cts.CTSText_TestUnit.parsable test would crash with the MissingRefsDecl exception and stop the whole testing process. I fixed this as follows. I think this fix is OK because if a text is not parsable, the self.xml.getroot() command will not complete. Thus, if the MissingRefsDecl exception is thrown, it means that the file was parsed as XML but errored in the ingestion into CapiTainS stage, which I think is OK. But any suggestions are welcome:

def parsable(self):
        """ Chacke that the text is parsable (as XML) and ingest it through MyCapytain then.

        .. note:: Override super(parsable) and add CapiTainS Ingesting to it
        """
        status = next(
            super(CTSText_TestUnit, self).parsable()
        )
        if status is True:
            try:
                self.Text = CapitainsCtsText(resource=self.xml.getroot())
            except MissingRefsDecl as E:
                yield status
        else:
            self.Text = None
        yield status

[sonofmun]

The function as it previously was starts at https://github.com/Capitains/HookTest/blob/master/HookTest/capitains_units/cts.py#L347

[sonofmun]

At this point, all unit tests are now passing. If the above solution is OK, then I will update CHANGES, bump the version number, and do a PR.

[sonofmun]

In the except clause, I should probably add self.Text = None.

[PonteIneptique]

My feeling is that yield in except MissingRefsDecl should give False. Can you explain to me why it should not ? (And I get that status = True here because of the if status is True up there)

[sonofmun]

Because the test is to see if the text is parsable as XML. Just because it doesn't have a refsDecl does not mean that it is not valid XML.

[sonofmun]

The problem that I have with this solution is that it doesn't end up pointing to the problem. The text is parsable but not ingestible in CapiTainS. I think the except clause should also trigger a failure of the refsDecl test. But since self.Text == None, no other tests will be run (or else they will all fail).

[sonofmun]

Here is my new solution:

def parsable(self):
        """ Chacke that the text is parsable (as XML) and ingest it through MyCapytain then.

        .. note:: Override super(parsable) and add CapiTainS Ingesting to it
        """
        status = next(
            super(CTSText_TestUnit, self).parsable()
        )
        if status is True:
            try:
                self.Text = CapitainsCtsText(resource=self.xml.getroot())
            except MissingRefsDecl as E:
                self.Text = None
                self.log(E)
        else:
            self.Text = None
        yield status

[sonofmun]

Of course, if CapiTainS ingesting is considered a part of being parsable, then it should return False. But the error should also show up in the log (self.log(E)).

[PonteIneptique]

I am going to say we should make it fail here, because it is file parsing and not MyCapytain Parsing or XML Parsing in the logs. The log part seems extremely important whatever the decision is :)

[sonofmun]

My thought with the log: we should add another category attribute to each unit called captain_errors and put it in there. That way it can be retrieved when printing to the console just as duplicates, forbiddens, etc. are. And, looking at this, I also need to print the empty references in the console as well.

[PonteIneptique]

I am fine with this addition

[sonofmun]

Fixed in #128