Unit tests for scripts/fetch_updates.py

[mingness]

There are two functions that could be unit tested:

• update_contribution
• log_broken

For now, we don't need to unit test process_contribution, it's more of a controller, and we won't test the main script

[amankumar100]

assign me this issue, i will resolve.

[SableRaf]

assign me this issue, i will resolve.

Thanks for your interest in working on this @amankumar100! Could you briefly describe how you would approach the unit tests?

Just a heads-up that this issue was originally more of a note to self by @mingness and it may not be ready to assign in its current draft state (I'll let @mingness clarify if necessary). See this comment for context.

[amankumar100]

I’d test fetch_updates.py by breaking out key logic, mocking I/O and requests, and covering core behavior with unit test.

[mingness]

Thanks @SableRaf , that's right, this work was originally just a quick note to remind myself of the work I'd intended to do myself, so the work hadn't been fully fleshed out and detailed.

@amankumar100 , since tests are in some ways an expression of intended behavior, and since I've not documented that anywhere, you'll be flying a little blind with this work, but the functionality in this file, I hope, is pretty self-explanatory. I've updated the description above, after some thought, to limit the work to just creating unit tests for two of the functions.

I'll assign this to you. Feel free to ask questions here. Otherwise, I can have more detailed discussions in the PR.

[SableRaf]

I’d test fetch_updates.py by breaking out key logic, mocking I/O and requests, and covering core behavior with unit test.

Thanks for the effort here. That said, I’m having a hard time seeing how your response connects to the specific points @mingness raised in the issue description or the code in fetch_updates.py.

Could you take another look and propose a more detailed approach?

For example, which three functions would you focus on testing, and what edge cases would you consider?

[amankumar100]

You can please review and merge, i have only done unit test for fetch_updates.py

[SableRaf]

You can please review and merge, i have only done unit test for fetch_updates.py

Hi @amankumar100. I appreciate your initiative in opening PR #90. At the same time, I want to be transparent: I asked a few questions earlier in the thread to understand your approach, but didn’t hear back before the PR was submitted. This is the second time we received a PR from you without waiting to be assigned, or responding to feedback.

Submitting work without a discussion, especially on draft issues like this one, adds extra work for maintainers and makes collaboration harder. It also doesn’t give you the best chance to make a strong contribution.

For now, I’m going to close the PR so we can finish scoping the work internally. If you’d like to contribute in the future, please look for issues labeled “good first issue” or “help wanted”, and take time to engage with the discussion (and getting assigned) before jumping in. Thanks.

Please be aware that if this pattern continues, we may need to limit your ability to contribute to the project. Thanks for your understanding.

[amankumar100]

You can please review and merge, i have only done unit test for fetch_updates.py

Hi @amankumar100. I appreciate your initiative in opening PR #90. At the same time, I want to be transparent: I asked a few questions earlier in the thread to understand your approach, but didn’t hear back before the PR was submitted. This is the second time we received a PR from you without waiting to be assigned, or responding to feedback.

Sir she had already assigned me, then only I made PR. Here is the proof:

Regarding your question about full approach, here is the answer:

• Removed all unnecessary patch() decorators and mock_open() usage since both functions are self-contained and don't perform I/O
• Used direct dictionary inputs to simulate contribution and props objects
• Added logic in test cases to validate side effects on the contribution dictionary
• Included sys.path.insert() to handle imports cleanly without modifying project structure

Outcome:

• Tests now pass successfully with pytest -v
• Validates that key contribution update and error-handling logic work independently
• Establishes a clean testing pattern for future coverage (e.g., process_contribution())

Now I guess, you are satisfied.

If anything else you want to know, please ask, I am always there to politely and respectfully answer your question. Thank you!

[mingness]

@SableRaf thank you for your comments - I agree with you that discussion always makes the work that will be done much clearer for everyone. I appreciate that you are trying to protect the time and energy of maintainers, and foster good communication on this open source project.

@amankumar100 thanks for you work on the issue - I will continue the discussion on the work in the PR.

[amankumar100]

@SableRaf thank you for your comments - I agree with you that discussion always makes the work that will be done much clearer for everyone. I appreciate that you are trying to protect the time and energy of maintainers, and foster good communication on this open source project.

If this contribution satisfies all conditions, please review and merge.
If you want any other part to be unit tested and rectified, then tell me or create another issue and assign me. I will resolve.