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

Fix NullPointerException and Incorrect Mocking in DefaultBranchBuildStatusProbeTest #577

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MurtazaSaherwala
Copy link

@MurtazaSaherwala MurtazaSaherwala commented Dec 27, 2024

Description

This pull request addresses the issue of NullPointerException occurring during the execution of DefaultBranchBuildStatusProbeTest. The issue arose due to improper mocking of nested calls in the test setup, particularly involving PagedIterable and its iterator(). The changes ensure proper mocking and setup for all dependencies, ensuring stable and accurate test execution.

Closes #363

Testing done

Submitter checklist

Preview Give feedback

@alecharp
Copy link
Collaborator

alecharp commented Jan 2, 2025

I'm sorry but I fail to see what you bring on top of the pull request #422. Have you simply copy the code from that pull request?

You should have made a pull request which would have fixed that one, to acknowledge the work of the other contributor from that other pull request.

@MurtazaSaherwala
Copy link
Author

hey @alecharp,
I tried to do that but couldn't mange to.. so had to resort to this

@krisstern
Copy link
Member

Hi @MurtazaSaherwala no worries, but could you please at least update the description of this PR to credit the original author of #422? Also, explain your motivation to replace that PR with this since that one has become stale in the description. Without these it would not be appropriate.

@alecharp
Copy link
Collaborator

alecharp commented Jan 6, 2025

I have to admit that I'm puzzled by this. The branch used for #422 is still up on @AayushSaini101 repository, you could have use his repository as a remote, fetch the objects from it, checkout the branch create your own from that, add your commits and create a pull request from that new branch. This would have been better.
I would accept a force push on this pull request if you were to redo this pull request history to include the initial commits.

Copy link
Contributor

@gounthar gounthar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution, @MurtazaSaherwala. 🙏
I agree with Adrien on this matter. We should definitely acknowledge the author of the original source code. There are a couple of methods we could consider for this acknowledgment:

One option would be to create a PR targeting the branch of the first PR's author. However, there's a risk that the author might not respond, especially since his original PR seems to have become stale.

Another approach, as suggested by Adrien, involves adding the first PR author's repository as a remote and building from there.

Admittedly, I'm not a Git expert, but Adrien is. I believe that together, we can guide you in proposing a new PR based on the initial author's PR.
Does that sound good to you?

@gounthar
Copy link
Contributor

gounthar commented Jan 7, 2025

  1. Add the original author's fork as a remote
    First, add the original author's fork as a remote to your local repository. This will allow you to fetch their branch.
    git remote add aayush https://github.com/AayushSaini101/plugin-health-scoring.git && git fetch --all
  2. Fetch the branch from the stale PR
    Fetch the branch (Issue-363) from the original author's fork:
    git fetch aayush Issue-363
  3. Create a new branch based on the stale PR
    Create a new branch in your local repository based on the fetched branch:
    git checkout -b my-new-branch aayush/Issue-363
  4. Push the new branch to your fork
    Push the new branch to your fork on GitHub:
    git push origin my-new-branch
  5. Make your changes and commit
    Now you can make your changes on top of the stale PR's code. When you commit your changes, the original author's commits will remain intact, and they will be credited in the commit history.

@alecharp alecharp marked this pull request as draft January 8, 2025 10:45
@alecharp
Copy link
Collaborator

@MurtazaSaherwala do you think you have time to follow the provided instructions to finish your pull request?

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.

Add a probe for failing builds on the default branch
4 participants