test: add stable leaderboard regression coverage

[srijnabhargav]

Tested

Summary

• add a deterministic end-to-end regression test for leaderboard stats and ranking endpoints
• use the Rocket.Chat gsoc2025final.json snapshot as fixed fixture input
• compare current server responses against a checked-in expected snapshot to guard stable production behavior

What this tests

This PR starts the current server code against fixture-backed config/data paths and verifies:

• GET /stats
• GET /rank
• GET /rank?parameter=openprs
• GET /rank?parameter=issues
• selected GET /rank?username=... lookups
• GET /contributor?rank=1&parameter=mergedprs

Why

This adds the baseline regression coverage requested before merging the backend error-handling PR, so we can confirm that leaderboard ranking and totals still match current stable behavior.

Test plan

• Run npm test
• Confirm the regression suite passes against the fixed Rocket.Chat snapshot
• Verify the test uses real server routes with refresh disabled for deterministic execution

AI-assisted contribution disclosure - Mostly AI-generate

[amitb0ra]

Hi @srijnabhargav, the earlier PR already covers /stats, /rank, and /contributor, along with all other server endpoints. Could you clarify what additional value this provides beyond what #127 already covers?

[srijnabhargav]

Hi @srijnabhargav, the earlier PR already covers /stats, /rank, and /contributor, along with all other server endpoints. Could you clarify what additional value this provides beyond what #127 already covers?

Hi @amitb0ra, thanks for the clarification. I checked the other pr (127) locally as well.

It looks broader in scope, but I dont think it provides the same thing as this PR yet.

The reason is:

• this PR is focused on the specific maintainer request: a deterministic regression baseline for stable leaderboard behavior using the Rocket.Chat gsoc2025final.json snapshot
• it compares live /stats, /rank, and /contributor responses against checked-in expected results, so future changes can be validated against a known stable output
• it runs in an isolated way using test-specific paths, without rewriting the repo’s real runtime files during the test

By contrast, the other pr (127) currently appears to:

• mutate real files like src/server/config.json, src/assets/data/data.json, src/assets/data/log.json, and src/server/admindata.json during test setup
• fail locally on checkout because the expected dataset path is not available on that branch, refer following screenshots
• include assertions that do not match the current server behavior on master (for example malformed JSON returning 400, and /login response shape assumptions)

So while it aims for broader endpoint coverage, my understanding is that this PR is addressing the narrower regression requirement that was explicitly requested before merging the error-handling changes.

I had already started working on this after the task was assigned to me, so I’m continuing with that requested scope here.

[amitb0ra]

Hi @srijnabhargav, thanks for the feedback — I've updated #127 to address the points you raised:

• File mutation — test: add e2e test suite #127 now uses env-var path overrides (LEADERBOARD_CONFIG_PATH, LEADERBOARD_DATA_PATH, etc.) with fixtures copied to a temp directory. No production files are touched.
• Dataset availability — gsoc2025final.json is on master (commit 30bb026). test: add e2e test suite #127 works on a clean checkout.
• Assertions for current behavior — I've included the minimal fixes for Util.post (400 on bad JSON), process.exit() removal in API.js, and the corresponding app.js call sites (same issues identified in fix: add log / output message notifying operator when Github token has expired #126) so all 49 tests pass on current master with zero skips.

#127 now covers every endpoint tested here (/stats, /rank, /contributor) plus /data, /config, /log, /login, /setStartDate, /setInterval, /setIncludedRepositories, /remove, unknown routes, invalid JSON handling, and cross-endpoint consistency — 49 tests, 0 fail.

[srijnabhargav]

Hi @srijnabhargav, thanks for the feedback — I've updated #127 to address the points you raised:

• File mutation — test: add e2e test suite #127 now uses env-var path overrides (LEADERBOARD_CONFIG_PATH, LEADERBOARD_DATA_PATH, etc.) with fixtures copied to a temp directory. No production files are touched.
• Dataset availability — gsoc2025final.json is on master (commit 30bb026). test: add e2e test suite #127 works on a clean checkout.
• Assertions for current behavior — I've included the minimal fixes for Util.post (400 on bad JSON), process.exit() removal in API.js, and the corresponding app.js call sites (same issues identified in fix: add explicit backend error handling for GitHub token and API failures #126) so all 49 tests pass on current master with zero skips.

#127 now covers every endpoint tested here (/stats, /rank, /contributor) plus /data, /config, /log, /login, /setStartDate, /setInterval, /setIncludedRepositories, /remove, unknown routes, invalid JSON handling, and cross-endpoint consistency — 49 tests, 0 fail.

Thanks for the update, @amitb0ra.

It looks like #127 has evolved quite a bit since I first reviewed it.

My PR is still focused on the specific regression-baseline request around stable leaderboard behavior using the Rocket.Chat snapshot and checked-in expected outputs. I had already started working on that after this task was assigned to me.

Since there is now overlap between the two PRs, it would be helpful for maintainers to clarify which direction they would prefer:

• a narrower regression-baseline PR focused on the requested leaderboard checks, or
• a broader end-to-end suite covering multiple server endpoints

I’m happy to align with the preferred direction, but I wanted to note that I had been actively working on this assigned scope already.

[Sing-Li]

@srijnabhargav I really like the simplicity and direct approach of this PR.

BUT - I am unable to run it. Can you please do a fresh checkout/clone of your PR and then run:

npm i
npm run test

It doesn't seem to work. Please figure out what is missing and add it to the README.

Also PLEASE specify the EXACT version of nodejs that you used to create and test 🙏

[srijnabhargav]

@srijnabhargav I really like the simplicity and direct approach of this PR.

BUT - I am unable to run it. Can you please do a fresh checkout/clone of your PR and then run:

npm i npm run test

It doesn't seem to work. Please figure out what is missing and add it to the README.

Also PLEASE specify the EXACT version of nodejs that you used to create and test 🙏

Hi @Sing-Li sir, thanks for checking this.

I reproduced the fresh-checkout issue and found the missing piece.

What was wrong:

• npm i at the repo root installs only the root dependencies
• but this regression test boots src/server/app.js, which also depends on packages declared under src/server/package.json
• on a clean checkout, npm run test therefore failed with Cannot find module 'socket.io' unless the src/server dependencies were installed as well

What I did:

• re-tested from a fresh local state
• confirmed the fix is to install the backend dependencies before running the test
• updated test/README.md with the exact reproducible steps and the exact Node.js version I used

Verified working steps:

npm i
npm --prefix src/server install
npm run test

[Sing-Li]

Thanks @srijnabhargav It does work now 👏

Now I have the same comment as for Amit --> modifying the code that you test in a test PR is not a good thing.

Amit is now separating env support into another PR.

Let's wait for him to finish the .env (dotenv) support.

[srijnabhargav]

Thanks @srijnabhargav It does work now 👏

Now I have the same comment as for Amit --> modifying the code that you test in a test PR is not a good thing.

Amit is now separating env support into another PR.

Let's wait for him to finish the .env (dotenv) support.

Yes Sir and Thank you for the review 😇

[Sing-Li]

@srijnabhargav please:

1. fix to work with the dotEnv environment (remove modified core code)
2. integrate into CI

Thanks I'll be merging this next.

[srijnabhargav]

@srijnabhargav please:

1. fix to work with the dotEnv environment (remove modified core code)
2. integrate into CI

Thanks I'll be merging this next.

Sir I have resolved the merge conflicts, I mean fixed the work with the dotEnv environment.

I will handle the CI integration in a new PR shortly. Thank you.

[srijnabhargav]

Sir, I have opened a new PR to integrate CI: #132

Please merge this one first so that only the CI-related changes appear in the other PR.

I can’t stack PRs because my work is on my fork rather than the upstream repository.

I opened #132 as a separate PR because the linter was failing, and fixing it required formatting changes in src/server/app.js. As you mentioned earlier, I wanted to keep those changes out of my current PR, so I created a new one.

Please let me know if any changes are needed.

[Sing-Li]

@srijnabhargav "making it work with dotEnv" means absolutely NO CHANGES to the base code.

So please remove all changes to any file under src/server

And fix the tests so they work.

Thanks.

[srijnabhargav]

Sorry sir and thank you for the clarification. I have updated the PR

What was wrong earlier:

• I had introduced test-related changes under src/server, which was not aligned with the already-merged dotenv support in master.
• That was unnecessary because the upstream server already supports env-based path/config overrides such as CONFIG_PATH, DATA_PATH, LOG_PATH, ADMINDATA_PATH, CONFIG_BACKUP_PATH, and SERVER_PORT.

What I changed now:

• Reverted the src/server changes so the PR does not rely on core code modifications.
• Updated the regression test to use the existing upstream env/dotenv model from the test side only.
• Kept the test isolated by wiring fixture paths through env vars and preventing the background refresh process from affecting the snapshot check.
• Updated test/README.md with the exact install and test steps.

Validation: ran following commands

npm i
npm --prefix src/server install
npm test

Result: the regression test passes on the current upstream server code.

[Sing-Li]

LGTM

[Sing-Li]

Thanks @srijnabhargav !

[srijnabhargav]

Thanks @srijnabhargav !

Thank you for the review sir 😇