Skip to content

bug(test): Fix short circuit bug, testcase snapshots and deprecated codecov action#544

Merged
lwasser merged 5 commits into
all-contributors:mainfrom
gerteck:fix/shortCircuitBugAndTestcases
Jan 23, 2026
Merged

bug(test): Fix short circuit bug, testcase snapshots and deprecated codecov action#544
lwasser merged 5 commits into
all-contributors:mainfrom
gerteck:fix/shortCircuitBugAndTestcases

Conversation

@gerteck

@gerteck gerteck commented Jan 20, 2026

Copy link
Copy Markdown
Contributor

What:

Fixes #530

Why:

Identified that the cause of the testcase failures were from PR #489, to address #468 , as it introduced a small logical error and also added a new testcase for the identified bug without updating the test snapshots.

How:

  1. Use of the logical bug pullCreated ||= await triggerActionAdd(...); within the for loop caused it to short circuit after the first contributor, as pullCreated is set to True after including the first contributor, and short circuit for subsequent iterations in the loop (causing it to not add subsequent contributors in the list).

    • Fixed the bug accordingly to not short-circuit
  2. Testcase snapshot outdated due to added case to account for the bug identified in the PR

  3. Testcase snapshot oudated due to change in logging from that PR i.e. log.info(No contributions for ${who});

Testcases were tested on Node v22.22.0 (LTS)

Additionally, update codecov workflow to use codecov-action instead of deprecated codecov npm package (v3.8.3), see codecov npm which is no longer functioning correctly in the CI environment.

Checklist:

  • Documentation
  • Ready to be merged
  • Added myself to contributors table.
    Bot Usage

Logical OR assignment was causing
short circuit and not run for each
contributor
@vercel

vercel Bot commented Jan 20, 2026

Copy link
Copy Markdown

@gerteck is attempting to deploy a commit to the All Contributors Team on Vercel.

A member of the Team first needs to authorize it.

@gerteck

gerteck commented Jan 20, 2026

Copy link
Copy Markdown
Contributor Author

Changes to testcase and logic passes all testcases:

Test Suites: 8 passed, 8 total
Tests:       66 passed, 66 total
Snapshots:   64 passed, 64 total
Time:        1.588 s, estimated 2 s
Ran all test suites.

@gerteck

gerteck commented Jan 22, 2026

Copy link
Copy Markdown
Contributor Author

Will take a look & investigate the failing CI tests!

Edit: Use codecov-action instead of deprecated codecov npm package (v3.8.3), see codecov npm which is no longer functioning correctly in the CI environment.

@gerteck gerteck changed the title Fix short circuit bug and testcases Fix short circuit bug, testcase snapshots and deprecated codecov action Jan 22, 2026
@codecov

codecov Bot commented Jan 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@a2054b6). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff            @@
##             main      #544   +/-   ##
========================================
  Coverage        ?   100.00%           
========================================
  Files           ?        16           
  Lines           ?       457           
  Branches        ?        57           
========================================
  Hits            ?       457           
  Misses          ?         0           
  Partials        ?         0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread .github/workflows/test.yml Outdated
@lwasser lwasser changed the title Fix short circuit bug, testcase snapshots and deprecated codecov action bug(test): Fix short circuit bug, testcase snapshots and deprecated codecov action Jan 23, 2026
@lwasser

lwasser commented Jan 23, 2026

Copy link
Copy Markdown
Member

@all-contributors please add @gerteck for tests, bug

@allcontributors

Copy link
Copy Markdown
Contributor

@lwasser

I've put up a pull request to add @gerteck! 🎉

lwasser pushed a commit that referenced this pull request Jan 23, 2026
Adds @gerteck as a contributor for test, bug.

This was requested by lwasser [in this
comment](#544 (comment))

[skip ci]

---------

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

@lwasser lwasser left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks great! Tests run well locally and now seem to loop through all contribs! Oddly, I don't think that feature actually works to add 2 people in one request to the bot. But that is for another day!

@lwasser
lwasser merged commit 6f040c2 into all-contributors:main Jan 23, 2026
4 of 5 checks passed
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.

🛠️ Repo: Integration test failures on main

2 participants