Skip to content

Revert "test: add tests for REPL custom evals" - #57793

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
tniessen:revert-1f7cfb7
Apr 8, 2025
Merged

Revert "test: add tests for REPL custom evals"#57793
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
tniessen:revert-1f7cfb7

Conversation

@tniessen

@tniessentniessen commented Apr 8, 2025

Copy link
Copy Markdown
Member

This reverts commit 1f7cfb7, which, as far as I can tell, was merged into the main branch despite relevant test failures.

The new test also started showing up in reliability reports immediately, see nodejs/reliability#1167.

Refs: #57691 (comment)

@tniessen
Revert "test: add tests for REPL custom evals"
10a3e43
This reverts commit 1f7cfb7, which was
merged into the main branch despite relevant test failures.
@tniessentniessen added the fast-track PRs that do not need to wait for 48 hours to land. label Apr 8, 2025
@tniessen
tniessen requested a review from aduh95April 8, 2025 14:13
@github-actions

github-actionsBot commented Apr 8, 2025

Copy link
Copy Markdown
Contributor

Fast-track has been requested by @tniessen. Please 👍 to approve.

@tniessentniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 8, 2025
@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. repl Issues and PRs related to the REPL subsystem. labels Apr 8, 2025
@github-actionsgithub-actionsBot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Apr 8, 2025
@github-actions

github-actionsBot commented Apr 8, 2025

Copy link
Copy Markdown
Contributor
Failed to start CI
 ⚠ No approving reviews found
✘ Refusing to run CI on potentially unsafe PR
https://github.com/nodejs/node/actions/runs/14335840440

@codecov

codecovBot commented Apr 8, 2025•
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.22%. Comparing base (1540fc6) to head (10a3e43).
Report is 7 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #57793 +/- ##
==========================================
- Coverage 90.23% 90.22% -0.02% 
==========================================
Files 630 630 Lines 185288 185289 +1 Branches 36344 36342 -2 ==========================================
- Hits 167203 167178 -25 - Misses 11006 11009 +3 - Partials 7079 7102 +23
Files with missing linesCoverage Δ
lib/repl.js94.91% <100.00%> (+<0.01%)⬆️

... and 24 files with indirect coverage changes

🚀 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.

@tniessentniessen added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Apr 8, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 8, 2025
@nodejs-github-bot

nodejs-github-bot commented Apr 8, 2025

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/66135/

@tniessentniessen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 8, 2025
BridgeAR
BridgeAR approved these changes Apr 8, 2025
View reviewed changes

@BridgeARBridgeAR 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.

RSLGTM

@nodejs-github-bot

nodejs-github-bot commented Apr 8, 2025

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/66142/

@tniessentniessen added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 8, 2025
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 8, 2025
@nodejs-github-bot
nodejs-github-bot merged commit a6cc240 into nodejs:mainApr 8, 2025
@nodejs-github-bot

nodejs-github-bot commented Apr 8, 2025

Copy link
Copy Markdown
Collaborator

Landed in a6cc240

@dario-piotrowiczdario-piotrowicz mentioned this pull request Apr 12, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request May 1, 2025
@tniessen@RafaelGSS
Revert "test: add tests for REPL custom evals"
ea5e60d
This reverts commit 1f7cfb7, which was
merged into the main branch despite relevant test failures.
PR-URL: #57793
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@RafaelGSSRafaelGSS mentioned this pull request May 1, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
@tniessen@RafaelGSS
Revert "test: add tests for REPL custom evals"
12f4124
This reverts commit 1f7cfb7, which was
merged into the main branch despite relevant test failures.
PR-URL: #57793
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
aduh95 pushed a commit that referenced this pull request May 6, 2025
@tniessen@aduh95
Revert "test: add tests for REPL custom evals"
03a949c
This reverts commit 1f7cfb7, which was
merged into the main branch despite relevant test failures.
PR-URL: #57793
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
RafaelGSS pushed a commit that referenced this pull request May 14, 2025
@tniessen@RafaelGSS
Revert "test: add tests for REPL custom evals"
be0d9f0
This reverts commit 1f7cfb7, which was
merged into the main branch despite relevant test failures.
PR-URL: #57793
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
aduh95 pushed a commit that referenced this pull request May 16, 2025
@tniessen@aduh95
Revert "test: add tests for REPL custom evals"
5a2d45e
This reverts commit 1f7cfb7, which was
merged into the main branch despite relevant test failures.
PR-URL: #57793
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@tniessen@aduh95
Revert "test: add tests for REPL custom evals"
1749cf6
This reverts commit 1f7cfb7, which was
merged into the main branch despite relevant test failures.
PR-URL: #57793
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@github-actionsgithub-actionsBot mentioned this pull request May 19, 2025
Merged
aduh95 pushed a commit that referenced this pull request May 19, 2025
@tniessen@aduh95
Revert "test: add tests for REPL custom evals"
c6a45a9
This reverts commit 1f7cfb7, which was
merged into the main branch despite relevant test failures.
PR-URL: #57793
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@ghostghost mentioned this pull request Jun 8, 2025
Closed
dario-piotrowicz added a commit to dario-piotrowicz/node that referenced this pull request Jun 11, 2025
@dario-piotrowicz
test: add tests for REPL custom evals
a5ef49b
this commit reintroduces the REPL custom eval tests that have
been introduced in nodejs#57691
but reverted in nodejs#57793
the tests turned out problematic before because `getReplOutput`,
the function used to return the repl output wasn't taking into
account that input processing and output emitting are asynchronous
operation can resolve with a small delay
the new implementation here replaces `getReplOutput` with
`getReplRunOutput` that resolves repl inputs by running them
and using the repl prompt as an indicator to when the input
processing has completed
nodejs-github-bot pushed a commit that referenced this pull request Jun 15, 2025
@dario-piotrowicz
test: add tests for REPL custom evals
d6dade5
this commit reintroduces the REPL custom eval tests that have
been introduced in #57691
but reverted in #57793
the tests turned out problematic before because `getReplOutput`,
the function used to return the repl output wasn't taking into
account that input processing and output emitting are asynchronous
operation can resolve with a small delay
the new implementation here replaces `getReplOutput` with
`getReplRunOutput` that resolves repl inputs by running them
and using the repl prompt as an indicator to when the input
processing has completed
PR-URL: #57850
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Jun 16, 2025
@dario-piotrowicz@targos
test: add tests for REPL custom evals
704b1fa
this commit reintroduces the REPL custom eval tests that have
been introduced in #57691
but reverted in #57793
the tests turned out problematic before because `getReplOutput`,
the function used to return the repl output wasn't taking into
account that input processing and output emitting are asynchronous
operation can resolve with a small delay
the new implementation here replaces `getReplOutput` with
`getReplRunOutput` that resolves repl inputs by running them
and using the repl prompt as an indicator to when the input
processing has completed
PR-URL: #57850
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
aduh95 pushed a commit that referenced this pull request Jul 21, 2025
@dario-piotrowicz@aduh95
test: add tests for REPL custom evals
ad8c6fc
this commit reintroduces the REPL custom eval tests that have
been introduced in #57691
but reverted in #57793
the tests turned out problematic before because `getReplOutput`,
the function used to return the repl output wasn't taking into
account that input processing and output emitting are asynchronous
operation can resolve with a small delay
the new implementation here replaces `getReplOutput` with
`getReplRunOutput` that resolves repl inputs by running them
and using the repl prompt as an indicator to when the input
processing has completed
PR-URL: #57850
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
aduh95 pushed a commit that referenced this pull request Jul 24, 2025
@dario-piotrowicz@aduh95
test: add tests for REPL custom evals
9d3e451
this commit reintroduces the REPL custom eval tests that have
been introduced in #57691
but reverted in #57793
the tests turned out problematic before because `getReplOutput`,
the function used to return the repl output wasn't taking into
account that input processing and output emitting are asynchronous
operation can resolve with a small delay
the new implementation here replaces `getReplOutput` with
`getReplRunOutput` that resolves repl inputs by running them
and using the repl prompt as an indicator to when the input
processing has completed
PR-URL: #57850
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpincalpincalpinca approved these changes
@cjihrigcjihrigcjihrig approved these changes
@richardlaurichardlaurichardlau approved these changes
@BridgeARBridgeARBridgeAR approved these changes
@aduh95aduh95aduh95 approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.fast-trackPRs that do not need to wait for 48 hours to land.needs-ciPRs that need a full CI run.replIssues and PRs related to the REPL subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@tniessen@nodejs-github-bot@lpinca@cjihrig@richardlau@BridgeAR@aduh95