Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test reporter does not take into account forked repos branch names #452

Open
j-mie6 opened this issue Jan 27, 2021 · 7 comments
Open

Test reporter does not take into account forked repos branch names #452

j-mie6 opened this issue Jan 27, 2021 · 7 comments

Comments

@j-mie6
Copy link

j-mie6 commented Jan 27, 2021

My repo has diff-coverage running on any PR. This has been working great for PRs made within my own repo, but we found today that it doesn't work on a PR from a forked PR. On closer inspection the problem is this:

image

Here we can see three reports from master. However, they are (from top to bottom):

  • Test coverage report due to merge from my repo master up to the forked PR to merge changes
  • Test coverage report running on my master after change
  • Test coverage report from the PR from forked repo

As you can see, there is no distinction between the three test coverage submissions: they all appear to be reporting about master. But there are two different masters here! Really it should be something like j-mie6/master and minut1bc/master like this:

image

Because of this, code climate isn't running the diff-coverage or total-coverage on the PR, which is causing a stall, as diff-coverage is required.

@j-mie6
Copy link
Author

j-mie6 commented Jan 27, 2021

It's possible that this is due to the action I'm using to upload. (see linked issue), but that would depend on whether or not codeclimate does distinguish between the repo origins regardless.

@efueger
Copy link
Member

efueger commented Jan 28, 2021

Hi @j-mie6 - got it!

What's happening here is that you're using identical CC_TEST_REPORTER_ID's in both repos' coverage setup.

If you want to view results for both repos at Code Climate, you'll need to:

  • add the minut1bc/Parsley repo to Code Climate
  • replace its CC_TEST_REPORTER_ID on this line with minut1bc/Parsley's ID

At that point you'll have 2 repos on Code Climate, each receiving coverage with their unique test reporter IDs.

Let me know if you have any questions! Here to help.

@efueger efueger closed this as completed Jan 28, 2021
@j-mie6
Copy link
Author

j-mie6 commented Jan 28, 2021

Hi @j-mie6 - got it!

What's happening here is that you're using identical CC_TEST_REPORTER_ID's in both repos' coverage setup.

If you want to view results for both repos at Code Climate, you'll need to:

  • add the minut1bc/Parsley repo to Code Climate
  • replace its CC_TEST_REPORTER_ID on this line with minut1bc/Parsley's ID

At that point you'll have 2 repos on Code Climate, each receiving coverage with their unique test reporter IDs.

Let me know if you have any questions! Here to help.

Hi @efueger, thanks for responding! Actually, that's not quite what we mean: we don't want the coverage to run on minut1bc/parsley. We want it to run purely on j-mie6/parsley on the PR. Otherwise everyone who wants to make a PR would need to register their fork on code climate, and change the test reporter ID in the CI to be able to get their PR merged. If we use separate branch names, this works fine, it just doesn't work if the branch names match up - in particular master is problematic, but two PRs with the same branch name can also conflict from two different forks. There needs to be a way of identifying that a test report generated by the CI of the original repo during a PR originated from a branch on a forked version of the repo. Ideally, the forked repo should not have to have the CI enabled or be registered on code climate if it doesn't want to be.

@efueger
Copy link
Member

efueger commented Jan 28, 2021

Thanks, @j-mie6.

Can you point me to one of those PRs where diff-cov caused the PR to stall?

@efueger efueger reopened this Jan 28, 2021
@j-mie6
Copy link
Author

j-mie6 commented Jan 28, 2021

I've since force merged them down, but we can make another for you. I'll let you know when we've done this!

@j-mie6
Copy link
Author

j-mie6 commented Jan 28, 2021

Hey! Here is an example:

PR on Parsley

The branch that it originated from is minut1bc/Parsley/master, and is merging into j-mie6/Parsley/master.

The test reporter log will be as in the first post, where both test coverage reports will be from master, and so code climate won't report the coverage diff (as it only has "one" of the reports) and so the PR is stuck waiting for a status that will never be reported.

Let me know if you need any more information!

@j-mie6
Copy link
Author

j-mie6 commented Mar 30, 2021

Any news on this @efueger?

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

No branches or pull requests

3 participants