Skip to content

fix: declare test helper variables#977

Open
999axel999 wants to merge 1 commit into
objectionary:masterfrom
999axel999:zerocracy/969-declare-test-helpers
Open

fix: declare test helper variables#977
999axel999 wants to merge 1 commit into
objectionary:masterfrom
999axel999:zerocracy/969-declare-test-helpers

Conversation

@999axel999

Copy link
Copy Markdown

Summary

  • Declare calls and opts inside test/commands/java/test_pipeline.js tests instead of leaking globals.
  • Declare numberOfPlaceholders inside the generate_comments loops.
  • Add regression checks proving the helpers no longer land on globalThis.

Closes #969

Tests

  • node ..\objectionary__eoc_fix\node_modules\mocha\bin\mocha.js test\commands\java\test_pipeline.js --timeout 1200000
  • node ..\objectionary__eoc_fix\node_modules\mocha\bin\mocha.js test\commands\test_generate_comments.js --grep "does not leak placeholder" --timeout 1200000
  • node ..\objectionary__eoc_fix\node_modules\eslint\bin\eslint.js test\commands\java\test_pipeline.js test\commands\test_generate_comments.js
  • git diff --check origin/master...HEAD

Note: full test/commands/test_generate_comments.js is not runnable in this local Windows shell because its helper spawns bare node via execSync, and that child command is not found in this environment. The new leak regression itself was run and passes.

Prepared with local automation assistance and reviewed before submission.

@999axel999 999axel999 marked this pull request as ready for review June 7, 2026 02:47

@Thayorns Thayorns left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@yegor256 this PR looks great, but unfortunately the issue is already closed.

it('fills output depending on the number of placeholders in the input code', (done) => {
const home = makeHome();
for (numberOfPlaceholders = 0; numberOfPlaceholders < 3; ++numberOfPlaceholders) {
for (let numberOfPlaceholders = 0; numberOfPlaceholders < 3; ++numberOfPlaceholders) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i would rather rename this variable to be less composite

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.

Undeclared variables in test/commands/java/test_pipeline.js causing potential global scope conflict

2 participants