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