Skip to content
This repository has been archived by the owner on Jul 26, 2024. It is now read-only.

Flicking when running all tests #3

Closed
Alizter opened this issue Mar 18, 2023 · 9 comments
Closed

Flicking when running all tests #3

Alizter opened this issue Mar 18, 2023 · 9 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@Alizter
Copy link

Alizter commented Mar 18, 2023

Hi,

Running the tests in the dune repo for example doesn't quite work:

Screencast.from.2023-03-18.23-24-16.webm

There appears to be a lot of flickering and it isn't obvious what is actually happening.

For reference, I have the OCaml platform extension also installed.

It would be nice if this extension used dune rpc also so that it can connect to a dune already in watch mode. At the moment, Dune disallows multiple dune processes in the same project due to one of them acquiring a lock.

@Release-Candidate
Copy link
Owner

Release-Candidate commented Mar 19, 2023

Hi and thanks for your bug report.

Running the tests in the dune repo for example doesn't quite work:

There appears to be a lot of flickering and it isn't obvious what is actually happening.

What is happening looks to me like that the extension is adding new tests to the Test Explorer, but somehow 'forgetting' (overwriting?) about already existing ones. Some kind of race condition when adding nodes to the tree.
I can kind of reproduce that with the dune sources, not when running tests but discovering them at startup or by pressing the Refresh Tests button.
To be sure, please add the log of the extensions in the Output tab, see Q: Where can I see the log of the extension?

It should look like in the first GIF on the README: the icons of all to be executed tests to change to that rotating indicator and changing to the test state icon (passed or failed) afterwards.

Btw. the long running test ./_build/default/test/expect-tests/fsevents/.fsevents_tests.inline-tests/inline_test_runner_fsevents_tests.exe I did delete to not having to wait that long when the extensions starts up and runs all inline test runners. I'm going to add a configuration value to be able to always ignore certain test runners and only run them manually.

It would be nice if this extension used dune rpc also so that it can connect to a dune already in watch mode. At the moment, Dune disallows multiple dune processes in the same project due to one of them acquiring a lock.

Yes I know about the lock (I 'work around' that by retrying to run dune until it succeeds locking - which doesn't work with Dune in watch mode), but didn't know about RPC. As soon as dune rpc exec works, I'll be able to maybe use it. Maybe, because my question is what happens if somebody stops the watching dune process while there are still clients (tests) running?
Btw. that leads to another question: why does dune not use a running Dune watch process automatically instead of complaining about the lock? Is this a planned feature or should I make a feature request?

@Release-Candidate Release-Candidate self-assigned this Mar 19, 2023
@Release-Candidate Release-Candidate added bug Something isn't working enhancement New feature or request labels Mar 19, 2023
@Release-Candidate
Copy link
Owner

Release-Candidate commented Mar 19, 2023

OK, found the problem: that is just me deleting the wrong groups when there are more than one inline test runners. That should be a fast fix.

@Alizter
Copy link
Author

Alizter commented Mar 19, 2023

Btw. that leads to another question: why does dune not use a running Dune watch process automatically instead of complaining about the lock? Is this a planned feature or should I make a feature request?

We are planning to make all Dune processes connect to an existing rpc server if one is running eventually. You can track the progress of that here: ocaml/dune#7308

@Alizter
Copy link
Author

Alizter commented Mar 19, 2023

Also one more subtle thing which is really only a problem for the Dune repo, is that we aren't using dune which is what the extension is using but rather the bootstrapped executable ./dune.exe for development. This probably causes an unnecessary rebuild.

@Release-Candidate
Copy link
Owner

Also one more subtle thing which is really only a problem for the Dune repo, is that we aren't using dune which is what the extension is using but rather the bootstrapped executable ./dune.exe for development. This probably causes an unnecessary rebuild.

I could add a configuration option to use a special path to dune instead of the one in the path of opam env. So a non-global dune could be used for users that don't have Opam in their path.

Btw. got it working with my fix now, just by getting two long running tests out of their way. I've got 5 failing test (of 365 tests in total without runners I couldn't build or deleted) is that OK?

But I cannot compile all tests by calling dune test by hand.

  • mel is not installable, because I use 5.0 - that expected
  • there are errors of JS complaining about stdlib's Mutex - am I missing some switch to turn that error off?

And some deprecated functions on MacOS

@Release-Candidate
Copy link
Owner

Version 0.4.0 fixes the problem.
You can also set the dune executable in the settings.

But the mapping of tests from the sources to the inline test runners does not work reliably, so you should let expectppx.discoverInSources disabled. To update the test tree when new tests have been added, please use the Refresh Tests button until I've fixed this.
See: Dune rules, library targets and inline test runners

@Release-Candidate
Copy link
Owner

And a remark: As I've just found the -list-test-names argument (I' making a PR to add this and other useful arguments to the PPX Inline Test README) there is a new version on its way, that no longer needs to run all tests to get the list of tests, so discovery should be much faster (like for Alcotest)

@Alizter
Copy link
Author

Alizter commented Mar 23, 2023

Excellent working really well! I like this extension a lot. Have you reached out to the ocaml platform folks? I think this would be a nice addition.

@Release-Candidate
Copy link
Owner

Great to hear!
I've actually talked with Thibaut Mattio at OCaml Discuss. My plan is to port it to OCaml and add both extensions to it, I just don't know when.

@Alizter Alizter closed this as completed Mar 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants