Skip to content

Conversation

@liam923
Copy link
Contributor

@liam923 liam923 commented Feb 17, 2025

I experienced a flaky failure of the test tests/test-dirs/parameters.t/run.t:

File "tests/test-dirs/parameters.t/run.t", line 1, characters 0-0:
-tests/test-dirs/parameters.t/run.t
+tests/test-dirs/parameters.t/run.t.corrected
File "tests/test-dirs/parameters.t/run.t", line 325, characters 0-1:
   end"
 
 This only compiles if the various ways of getting at [P.t] are equivalent:
 
   $ $OCAMLC -bin-annot-cms -c use_reexported.mli use_reexported.ml -parameter P
 
 This is the function [check_that_types_are_the_same]:
 
   $ query_type use_reexported.ml 1:5 -parameter P
   "Reexport.As_alias.t -> Reexport.Included.t -> P.t"
 
 This is a reference to [Reexport.As_alias.create]:
 
   $ multi_query use_reexported.ml 6:30 -parameter P
   "unit -> Reexport.As_alias.t"
   "Make a thing."
-  {
-    "file": "$TESTCASE_ROOT/p.mli",
-    "pos": {
-      "line": 7,
-      "col": 4
-    }
-  }
+  abnormal termination
+  merlin path: _build/default/src/frontend/ocamlmerlin/ocamlmerlin_server.exe
+  socket path: /tmp/build_775917_dune/ocamlmerlin_23748_64773_4311858554.socket
+  "abnormal termination"
 
 And this goes to [Reexport.Included.create]:
 
   $ multi_query use_reexported.ml 7:30 -parameter P
   "unit -> Reexport.Included.t"
   "Make a thing."
   {
     "file": "$TESTCASE_ROOT/p.mli",
     "pos": {
       "line": 7,
       "col": 4
     }
   }
 
 Now let's try instantiating things.

Here's a possible explanation of the flake: I experienced this flake twice, and both times were immediately after pulling changes. Currently, the test uses the merlin server but never stops the server. The flaky run could've been using a server built from the codebase before pulling, which would cause unexpected behavior.

I am not confident of this explanation because I have been unable to replicate the flake by trying to replicate the circumstances. But either way, it is a good idea to stop the server before and after the test (all other tests that use the merlin server do this). So this PR does so. If the flake keeps occurring, then we will revisit the issue.

@liam923 liam923 requested a review from lukemaurer February 17, 2025 19:45
Copy link
Contributor

@lukemaurer lukemaurer left a comment

Choose a reason for hiding this comment

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

LGTM

@liam923 liam923 merged commit 4336bc1 into main Feb 17, 2025
1 of 2 checks passed
@liam923 liam923 deleted the fix-parameter-flake branch February 17, 2025 20:00
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.

3 participants