Skip to content

Conversation

ElectreAAS
Copy link
Collaborator

@ElectreAAS ElectreAAS commented Sep 17, 2025

Contributed to #8114
Supersedes #11900.

This PR is a 'remake' of Steve's: I rebased it on main, and applied what I learned making #12010 and #12064. This results in significant changes from the original, hence the new PR.

The 'runtest' RPC request is now public, in otherlibs. runtest_rpc.ml{i} was removed.

The behaviour is now (AFAI know what I'm doing) identical between running dune runtest [ARGUMENTS] with or without the RPC server.

See comments below for details

@ElectreAAS ElectreAAS force-pushed the test-concurrent-with-watch-mode branch from fec9cf9 to f20ddb3 Compare September 17, 2025 17:47
@ElectreAAS
Copy link
Collaborator Author

Notes from rebasing: Since the contents of bin/runtest.ml have been largely moved to bin/runtest_common.ml, it'd be easy to accidentaly overwrite changes made to the former since the original PR.
#11869 and #12251 are PRs that fit that description, and have been correctly applied to runtest_common

@ElectreAAS ElectreAAS requested a review from Alizter September 17, 2025 17:51
@ElectreAAS ElectreAAS force-pushed the test-concurrent-with-watch-mode branch from 009a1ff to ccc3909 Compare September 30, 2025 09:54
@ElectreAAS ElectreAAS force-pushed the test-concurrent-with-watch-mode branch from c4ba736 to 27412bc Compare October 2, 2025 18:37
@Alizter Alizter self-assigned this Oct 3, 2025
@ElectreAAS
Copy link
Collaborator Author

ElectreAAS commented Oct 3, 2025

Outdated

The last commit (27412bc) is maybe a bit overzealous in the refactoring, but that was the only way I found to make sure the error messages were correct.
Review can be done without it, and if deemed necessary I could put it in a different PR

Up to date

I just force-pushed a few changes:

  • it's now rebased on main
  • there are now only 2 commits:
    • the first is the one that has only what is necessary for the feature, without previous refactorings
    • the second has all the plumbing required to make the 'forwarding' warning appear at the correct time (after actually connecting to the RPC server)

It's now both ready to review and reviewable! 😃

PS: the previous state of this PR is still visible on my fork, as a backup

@ElectreAAS ElectreAAS force-pushed the test-concurrent-with-watch-mode branch from 27412bc to 7d000a2 Compare October 7, 2025 10:17
@shonfeder shonfeder requested a review from gridbugs October 7, 2025 22:12
@ElectreAAS ElectreAAS force-pushed the test-concurrent-with-watch-mode branch from 7d000a2 to 6cb1657 Compare October 13, 2025 16:13
@Alizter Alizter requested a review from rgrinberg October 13, 2025 19:29
@Alizter Alizter force-pushed the test-concurrent-with-watch-mode branch from 6cb1657 to 4a28f9c Compare October 13, 2025 19:30
@ElectreAAS ElectreAAS merged commit 7861fb5 into ocaml:main Oct 14, 2025
11 of 12 checks passed
@ElectreAAS ElectreAAS deleted the test-concurrent-with-watch-mode branch October 14, 2025 09:54
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.

2 participants