Skip to content

ci: add codecov.io support to flux#1273

Merged
jsternberg merged 1 commit intomasterfrom
refactor/codecov-ci
May 16, 2019
Merged

ci: add codecov.io support to flux#1273
jsternberg merged 1 commit intomasterfrom
refactor/codecov-ci

Conversation

@jsternberg
Copy link
Copy Markdown
Contributor

@jsternberg jsternberg commented May 13, 2019

  • docs/SPEC.md updated
  • Test cases written

Fixes #1094.

@jsternberg jsternberg force-pushed the refactor/codecov-ci branch from 50927dc to 734d6fd Compare May 13, 2019 18:33
@jsternberg
Copy link
Copy Markdown
Contributor Author

Here's the output showing this works: https://codecov.io/gh/influxdata/flux/branch/refactor%2Fcodecov-ci

It won't paste anything into the issue until after it is merged to master. Once it is merged to master, it'll show a comparison for every pull request.

@jsternberg jsternberg requested a review from nathanielc May 13, 2019 18:34
@codecov-io
Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (master@3a467e7). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #1273   +/-   ##
========================================
  Coverage          ?   54.4%           
========================================
  Files             ?     179           
  Lines             ?   27396           
  Branches          ?       0           
========================================
  Hits              ?   14905           
  Misses            ?   11073           
  Partials          ?    1418

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a467e7...734d6fd. Read the comment docs.

@nathanielc
Copy link
Copy Markdown
Contributor

@jsternberg How do we use this report when reviewing PRs? I get that the report is hard to read right now because there is no baseline to compare against, but once we do, how do you intend for use to consume this information?

Is it that we won't merge PRs that reduce code coverage? Is it just informational to help us think about test cases? What are the goals of adding code coverage?

@jsternberg
Copy link
Copy Markdown
Contributor Author

I want it so we don't merge PRs unless they increase code coverage, but it's obviously flexible. I think the goal of adding code coverage is to give us a signal that we didn't add tests or, if we did add tests, we didn't add tests in places that we thought we might have.

At the moment our coverage seems to be at 54%. If we can get that up to above 80% and have it stay there, I think that will increase our test quality.

@nathanielc
Copy link
Copy Markdown
Contributor

I see that there are two checks associated with codecov codecov/patch and codecov/project what do those do?

Is it possible to have the check be green on increased code coverage and yellow on decreased coverage? If so is it possible to have the code coverage report linked from those health check details instead of as a comment?

@jsternberg
Copy link
Copy Markdown
Contributor Author

I had never seen that, but apparently yes. Here is the configuration file for it: https://docs.codecov.io/docs/codecov-yaml

I think this might be what you're specifically looking for: https://docs.codecov.io/docs/commit-status

@nathanielc
Copy link
Copy Markdown
Contributor

Yes, lets try this out, what I would like to see is a github bot integration that acts a little like the semantic commit message bot. It never goes red, but will be yellow if the coverage goes down. That way it becomes a good reminder and positive feedback, but it doesn't prevent merged. That said, this will likely all change once we switch to the only bots merge to master approach. So lets go forward with this as is.

@jsternberg jsternberg merged commit 3204dd2 into master May 16, 2019
@jsternberg jsternberg deleted the refactor/codecov-ci branch May 16, 2019 21:31
jsternberg added a commit that referenced this pull request Jun 2, 2019
When the git log contained merges, the changelog generator didn't
necessarily work correctly. First, it would attempt to parse the message
in merge commits which was always wrong and always printed an error
message. Now, the changelog generator is told to just skip those commits
and move on since merge commits won't contain the message we're looking
for anyway.

The second happened with patch releases and feature branches that were
under work for more than a release. If a release was performed on the
maint branch and another commit was done on master before merging
maint back to master, the tree could be constructed in a way where the
previous version commit could not be clearly found such as in this
example from the `v0.31.0` release.

    * 3204dd2 ci: add codecov.io support to flux (#1273)
    *   f0b6850 Merge branch 'maint'
    |\
    | * ec37117 (tag: v0.30.0) feat(universe): support for dynamic queries (#1284)
    * | 2cce6fe feat(universe): support for dynamic queries (#1284)
    |/
    * 32fb2fb (tag: v0.29.0) fix(execute): properly use RefCount to reference count tables (#1216)

If the changelog generator encountered this pattern, it was undefined
exactly how it would behave. In this specific scenario, it parsed every
commit on the left branch going back to the root commit and then found
the previous commit on the second side of the branch. The opposite could
happen too and then it would have missed adding the commit from the
other branch.

The changelog generator has now been modified to find the merge base
before it attempts to read the log to prevent searching back through the
entire history. In the above situations, `v0.29.0` would be found and
act as a final constraint for parent commits.

The changelog parser has also been modified to use a breadth-first
search instead of depth-first search and it does not use the `Log()`
method from the go-git library. This is because the go-git library does
not have a way to stop at a specific commit and stop looking at parent
commits. We now limit the breadth-first search of commits to either the
merge base or the previous version commit which should be the same
commit if the release process was followed correctly. A breadth-first
search is used to ensure we get all commit messages from any merges and
don't skip any commits because we reached the terminating commit first.
jsternberg added a commit that referenced this pull request Jun 2, 2019
When the git log contained merges, the changelog generator didn't
necessarily work correctly. First, it would attempt to parse the message
in merge commits which was always wrong and always printed an error
message. Now, the changelog generator is told to just skip those commits
and move on since merge commits won't contain the message we're looking
for anyway.

The second happened with patch releases and feature branches that were
under work for more than a release. If a release was performed on the
maint branch and another commit was done on master before merging
maint back to master, the tree could be constructed in a way where the
previous version commit could not be clearly found such as in this
example from the `v0.31.0` release.

    * 3204dd2 ci: add codecov.io support to flux (#1273)
    *   f0b6850 Merge branch 'maint'
    |\
    | * ec37117 (tag: v0.30.0) feat(universe): support for dynamic queries (#1284)
    * | 2cce6fe feat(universe): support for dynamic queries (#1284)
    |/
    * 32fb2fb (tag: v0.29.0) fix(execute): properly use RefCount to reference count tables (#1216)

If the changelog generator encountered this pattern, it was undefined
exactly how it would behave. In this specific scenario, it parsed every
commit on the left branch going back to the root commit and then found
the previous commit on the second side of the branch. The opposite could
happen too and then it would have missed adding the commit from the
other branch.

The changelog generator has now been modified to find the merge base
before it attempts to read the log to prevent searching back through the
entire history. In the above situations, `v0.29.0` would be found and
act as a final constraint for parent commits.

The changelog parser has also been modified to use a breadth-first
search instead of depth-first search and it does not use the `Log()`
method from the go-git library. This is because the go-git library does
not have a way to stop at a specific commit and stop looking at parent
commits. We now limit the breadth-first search of commits to either the
merge base or the previous version commit which should be the same
commit if the release process was followed correctly. A breadth-first
search is used to ensure we get all commit messages from any merges and
don't skip any commits because we reached the terminating commit first.
jsternberg added a commit that referenced this pull request Jun 3, 2019
When the git log contained merges, the changelog generator didn't
necessarily work correctly. First, it would attempt to parse the message
in merge commits which was always wrong and always printed an error
message. Now, the changelog generator is told to just skip those commits
and move on since merge commits won't contain the message we're looking
for anyway.

The second happened with patch releases and feature branches that were
under work for more than a release. If a release was performed on the
maint branch and another commit was done on master before merging
maint back to master, the tree could be constructed in a way where the
previous version commit could not be clearly found such as in this
example from the `v0.31.0` release.

    * 3204dd2 ci: add codecov.io support to flux (#1273)
    *   f0b6850 Merge branch 'maint'
    |\
    | * ec37117 (tag: v0.30.0) feat(universe): support for dynamic queries (#1284)
    * | 2cce6fe feat(universe): support for dynamic queries (#1284)
    |/
    * 32fb2fb (tag: v0.29.0) fix(execute): properly use RefCount to reference count tables (#1216)

If the changelog generator encountered this pattern, it was undefined
exactly how it would behave. In this specific scenario, it parsed every
commit on the left branch going back to the root commit and then found
the previous commit on the second side of the branch. The opposite could
happen too and then it would have missed adding the commit from the
other branch.

The changelog generator has now been modified to find the merge base
before it attempts to read the log to prevent searching back through the
entire history. In the above situations, `v0.29.0` would be found and
act as a final constraint for parent commits.

The changelog parser has also been modified to use a breadth-first
search instead of depth-first search and it does not use the `Log()`
method from the go-git library. This is because the go-git library does
not have a way to stop at a specific commit and stop looking at parent
commits. We now limit the breadth-first search of commits to either the
merge base or the previous version commit which should be the same
commit if the release process was followed correctly. A breadth-first
search is used to ensure we get all commit messages from any merges and
don't skip any commits because we reached the terminating commit first.
jsternberg added a commit that referenced this pull request Jun 3, 2019
…1319)

When the git log contained merges, the changelog generator didn't
necessarily work correctly. First, it would attempt to parse the message
in merge commits which was always wrong and always printed an error
message. Now, the changelog generator is told to just skip those commits
and move on since merge commits won't contain the message we're looking
for anyway.

The second happened with patch releases and feature branches that were
under work for more than a release. If a release was performed on the
maint branch and another commit was done on master before merging
maint back to master, the tree could be constructed in a way where the
previous version commit could not be clearly found such as in this
example from the `v0.31.0` release.

    * 3204dd2 ci: add codecov.io support to flux (#1273)
    *   f0b6850 Merge branch 'maint'
    |\
    | * ec37117 (tag: v0.30.0) feat(universe): support for dynamic queries (#1284)
    * | 2cce6fe feat(universe): support for dynamic queries (#1284)
    |/
    * 32fb2fb (tag: v0.29.0) fix(execute): properly use RefCount to reference count tables (#1216)

If the changelog generator encountered this pattern, it was undefined
exactly how it would behave. In this specific scenario, it parsed every
commit on the left branch going back to the root commit and then found
the previous commit on the second side of the branch. The opposite could
happen too and then it would have missed adding the commit from the
other branch.

The changelog generator has now been modified to find the merge base
before it attempts to read the log to prevent searching back through the
entire history. In the above situations, `v0.29.0` would be found and
act as a final constraint for parent commits.

The changelog parser has also been modified to use a breadth-first
search instead of depth-first search and it does not use the `Log()`
method from the go-git library. This is because the go-git library does
not have a way to stop at a specific commit and stop looking at parent
commits. We now limit the breadth-first search of commits to either the
merge base or the previous version commit which should be the same
commit if the release process was followed correctly. A breadth-first
search is used to ensure we get all commit messages from any merges and
don't skip any commits because we reached the terminating commit first.
davidgs pushed a commit to davidgs/flux that referenced this pull request Aug 20, 2019
davidgs pushed a commit to davidgs/flux that referenced this pull request Aug 20, 2019
…nfluxdata#1319)

When the git log contained merges, the changelog generator didn't
necessarily work correctly. First, it would attempt to parse the message
in merge commits which was always wrong and always printed an error
message. Now, the changelog generator is told to just skip those commits
and move on since merge commits won't contain the message we're looking
for anyway.

The second happened with patch releases and feature branches that were
under work for more than a release. If a release was performed on the
maint branch and another commit was done on master before merging
maint back to master, the tree could be constructed in a way where the
previous version commit could not be clearly found such as in this
example from the `v0.31.0` release.

    * 3204dd2 ci: add codecov.io support to flux (influxdata#1273)
    *   f0b6850 Merge branch 'maint'
    |\
    | * ec37117 (tag: v0.30.0) feat(universe): support for dynamic queries (influxdata#1284)
    * | 2cce6fe feat(universe): support for dynamic queries (influxdata#1284)
    |/
    * 32fb2fb (tag: v0.29.0) fix(execute): properly use RefCount to reference count tables (influxdata#1216)

If the changelog generator encountered this pattern, it was undefined
exactly how it would behave. In this specific scenario, it parsed every
commit on the left branch going back to the root commit and then found
the previous commit on the second side of the branch. The opposite could
happen too and then it would have missed adding the commit from the
other branch.

The changelog generator has now been modified to find the merge base
before it attempts to read the log to prevent searching back through the
entire history. In the above situations, `v0.29.0` would be found and
act as a final constraint for parent commits.

The changelog parser has also been modified to use a breadth-first
search instead of depth-first search and it does not use the `Log()`
method from the go-git library. This is because the go-git library does
not have a way to stop at a specific commit and stop looking at parent
commits. We now limit the breadth-first search of commits to either the
merge base or the previous version commit which should be the same
commit if the release process was followed correctly. A breadth-first
search is used to ensure we get all commit messages from any merges and
don't skip any commits because we reached the terminating commit first.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integrate code coverage in the CI process

3 participants