percy / python-percy-client

[Deprecated] Python client library for visual regression testing with Percy.
https://percy.io/docs/clients/python/selenium
MIT License
21 stars 24 forks source link

Update python Percy client to use env data when git isn't around #38

Closed cadeParade closed 6 years ago

cadeParade commented 6 years ago

Refactor commit method to:

Update tests to test specifically for data in git/environment specifically.

cadeParade commented 6 years ago

I respect your personal preferences ;)

What do you think about something like this?

    @property
    def commit_data(self):
        # If not running in a git repo, allow undefined for certain commit attributes.
        def parse(git_data, regex):
            if not git_data:
                return None
            match = regex.search(git_data)
            if match:
                return match.group(1)
            else:
                return None

        def get_data_with_fallback(git_data, key):
            # Try reading git information, fallback on env vars
            regex_string = re.compile("%s:(.*)" % (key))
            env_var_name = "GIT_%s" % (key)
            data_from_git = parse(git_data, regex_string)
            return data_from_git or os.getenv(env_var_name)

        # Try getting data from git
        raw_git_output = self._git_commit_output()

        return {
            # The only required attribute:
            'branch': self.branch,
            # An optional but important attribute:
            'sha': self.commit_sha or get_data_with_fallback(raw_git_output, "COMMIT_SHA"),

            # Optional attributes:
            'message': get_data_with_fallback(raw_git_output, "COMMIT_MESSAGE"),
            'committed_at': get_data_with_fallback(raw_git_output, "COMMITTED_DATE"),
            'author_name': get_data_with_fallback(raw_git_output, "AUTHOR_NAME"),
            'author_email': get_data_with_fallback(raw_git_output, "AUTHOR_EMAIL"),
            'committer_name': get_data_with_fallback(raw_git_output, "COMMITTER_NAME"),
            'committer_email': get_data_with_fallback(raw_git_output, "COMMITTER_EMAIL"),
        }

Am I trying too hard? I just found it very hard to reason about what was going on in this method when I started, so I'm trying to make it easier to grok. But also we all have our own style, so lmk if you think this is too much.

timhaines commented 6 years ago

@cadeParade I'd prefer what you have over this - current PR code is definitely easier to grok. Also, probably not pertinent, but the message regex needs the flags=re.MULTILINE flag.