-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
repl: refactor tests to not rely on timing #17828
Conversation
@nodejs/testing @addaleax @Fishrock123 @princejwesley |
@bmeck this needs a rebase, thanks! |
Tests relying on synchronous timing have been migrated to use events.
bbe4925
to
01e697e
Compare
Landed in de848ac 🎉 |
Tests relying on synchronous timing have been migrated to use events. PR-URL: nodejs#17828 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
Hm, this broke multilines in the This can easily be verified with:
After entering that, it is not possible to type anything anymore. @bmeck do you think you can get this working soon again? Otherwise I guess the best will be to revert it? |
@BridgeAR I'll take a look tomorrow and think I can fix it. |
This reverts commit de848ac. The commit broke multiline repl. PR-URL: nodejs#18715 Refs: nodejs#17828 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit adds a regression test for de848ac, which broke multiline input in the REPL. PR-URL: nodejs#18718 Refs: nodejs#17828 Refs: nodejs#18715 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
edit: looks like it was reverted setting don't land |
Tests relying on synchronous timing have been migrated to use events. PR-URL: nodejs#17828 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
This reverts commit de848ac. The commit broke multiline repl. PR-URL: nodejs#18715 Refs: nodejs#17828 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit adds a regression test for de848ac, which broke multiline input in the REPL. PR-URL: nodejs#18718 Refs: nodejs#17828 Refs: nodejs#18715 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Tests relying on synchronous timing have been migrated to use events.
This is in preparation for more robust testing with the top level await transform. Some minor naming changes were also made to try and make bits more consistent as I was reading them.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
repl, test