1. Ned Batchelder
  2. coverage.py
  3. Issues
Issue #258 invalid

Reading incorrect file via COVERAGE_PROCESS_START

Paul Sargent
created an issue

I've just been debugging an issue I've been getting running the Mercurial test suite with coverage enabled. The symptom was that when the environment variable COVERAGE_PROCESS_START (CPS) was set to a filename, I'd get import errors in the test output saying:

'import sitecustomize' failed; use -v for traceback

sitecustomize.py was being used to call process_startup(). This extra output was causing tests to fail. I'm running Coverage 3.6 on Python 2.7.5

What I discovered was happening is that process_startup() requests the value of CPS and passes it to the coverage. This value (in 2.7.5) is a <str>

    cps = os.environ.get("COVERAGE_PROCESS_START")
    if cps:
        cov = coverage(config_file=cps, auto_data=True)

As config_file is not True, coverage.__init__() tries to use it as a file-name and passes it to config.from_file()

        # 2: from the coveragerc file:
        if config_file:
            if config_file is True:
                config_file = ".coveragerc"
            try:
                self.config.from_file(config_file)

Eventually we get to HandyConfigParser.read which passes the filename to RawConfigParser.read()

class HandyConfigParser(configparser.RawConfigParser):
    """Our specialization of ConfigParser."""

    def read(self, filename):
        """Read a filename as UTF-8 configuration data."""
        kwargs = {}
        if sys.version_info >= (3, 2):
            kwargs['encoding'] = "utf-8"
        return configparser.RawConfigParser.read(self, filename, **kwargs)

Unfortunately, RawConfigParser does something which I think is a little odd (in 2.7.5). It checks if the filenames parameter is of type <unicode> and if not, it treats the the parameter as a list of filenames.

    def read(self, filenames, encoding=None):
        u"""Read and parse a filename or a list of filenames.

        Files that cannot be opened are silently ignored; this is
        designed so that you can specify a list of potential
        configuration file locations (e.g. current directory, user's
        home directory, systemwide directory), and all existing
        configuration files in the list will be read.  A single
        filename may also be given.

        Return list of successfully read files.
        """
        if isinstance(filenames, unicode):
            filenames = [filenames]
        read_ok = []
        for filename in filenames:
            try:
                with open(filename, encoding=encoding) as fp:
                    self._read(fp, filename)
            except IOError:
                continue
            read_ok.append(filename)
        return read_ok

In our case filenames is the <str> we got from the CPS environment variable, and so RawConfigParser starts to iterate over the characters of filenames, and tries to open files with single character names.

In the case of the Mercurial test suite it would often find such files because test files with names like a, b or c are common, and those characters are common in the paths that CPS is set to.

Having found such a file, it would then try to be parsed as a configuration file, and an exception would be raised when the format was wrong.

My fix is to change HandyConfigParser to place the filename into a list. It's then interpreted correctly by RawConfigParser

class HandyConfigParser(configparser.RawConfigParser):
    """Our specialization of ConfigParser."""

    def read(self, filename):
        """Read a filename as UTF-8 configuration data."""
        kwargs = {}
        if sys.version_info >= (3, 2):
            kwargs['encoding'] = "utf-8"
        return configparser.RawConfigParser.read(self, [filename], **kwargs)
                                                       ^^^ Here ^^^

Comments (8)

  1. Ned Batchelder repo owner

    Paul Sargent thanks for the detailed debugging, and the patch. I'm confused though: COVERAGE_PROCESS_START is always set to a file name, and has been working fine for lots of people. Your debugging clearly shows this to be an issue, but how come others (including me) aren't seeing a problem? I think people have had their configuration file read properly from COVERAGE_PROCESS_START. Could it be we've all been mistaken?

  2. Ned Batchelder repo owner

    Paul Sargent For example, the test tests/test_process.py:ProcessStartupTest:test_subprocess_with_pth_files will only pass if the config file specified in COVERAGE_PROCESS_START is properly read. There's more to this story...

  3. Ned Batchelder repo owner

    Paul Sargent My source for 2.7.5 has this at the start of RawConfigParser.read:

            if isinstance(filenames, basestring):
                filenames = [filenames]
    

    Where did your file come from? Yours definitely seems wrong, but it also seems different than the one distributed. Also, your docstring is a u"" string, mine is not.

    In py3.3, the line in question is if isinstance(filename, str):, your file looks oddly like the result of 3to2 on the py3.3 version of the file.

  4. Paul Sargent reporter

    It looks that way to me, but maybe there's been a change to the behaviour of the stdlib as its tried to become Unicode aware. The real bizare thing is for the stdlib to check for a Unicode filename and not a byte string one.

    ...and because it normally fails silently, nobody has noticed.

  5. Log in to comment