Test for the interface

[nebadesmondc]

Hello, I wanted to provide an update on my progress regarding the task mentioned in issue #9 . I have implemented a proposed solution for the issue and would appreciate a review.

@Julow @panglesd

[nebadesmondc]

I have implemented some of the requested changes. @Julow

[nebadesmondc]

However, it prints nothing. Also, how to send input events to it ? I would like to write different invocations from the cram test that simulate different

I noticed this and haven't found a way around it yet. Any help on how I can fix this?

[Julow]

It prints nothing because open_dummy_terminal suppresses all output. I suggest removing open_dummy_terminal for now.

[nebadesmondc]

I have removed open_dummy_terminal. Somehow making it take inputs from the command line is still a challenge @Julow

[Julow]

The command line is defined using the cmdliner library, Arg.pos_all will take all the arguments into a list, use it instead of const (). Good luck ;)

[nebadesmondc]

I have made some changes such that it now accepts input from the command line

[nebadesmondc]

Fixed: (cram (deps %{bin:dummy_terminal}))

[nebadesmondc]

I have done some changes. But the pull request check fails at the test level with the error: Error: Process completed with exit code 1.

What next? @Julow

[Julow]

Run the tests with opam exec -- dune runtest --auto-promote to make sure that the cram test reflects the current output.

End_of_file is an exception from the standard library that is usually raised when reading past the end of a file. It could be raised by input_char.

Can you explain why you are using Unix.select and input_char ? I'd expect that no IO is needed as the inputs are already all in a list.

[nebadesmondc]

Yeah that's right. I have made come changes.

[Julow]

Nice! It would be nice to send several inputs and see whether we still see only the last image.

[nebadesmondc]

I have fixed that.

[nebadesmondc]

I am not sure of how to go about that. Some directions and guidance from you would be immensely helpful in getting me started.

[Julow]

It's your job to come up with a plan. Start by reading and understanding the issue and the code I linked to.

[Julow]

Congrat on the progress made so far!

[nebadesmondc]

I have modified the Interactive_viewer.start and made some other changes to enable running if the application @Julow

[Julow]

Can you explain what your last changes is doing ? Notably, how is it going to be used from the cram tests.

[nebadesmondc]

My last change includes the modification of the Interactive_viewer.start to make it accepts events(custom inputs), I then modified the interactive_viewer_dummy.ml to be able to run the application. I was able to run test this using the command ./_build/default/test/cram/interactive_viewer_dummy.exe example.diff q but I have not succeeded in running the cram test, I encountered this error while running (opam exec -- dune runtest)

#Run the executable $ dummy_terminal example.diff +diffcessible: internal error, uncaught exception: +Sys_error("example.diff: No such file or directory") +[125]

[Julow]

It seems that the program is trying to open a file named example.diff but do not succeed. Try creating such a file in the cram test. Doc about directory cram tests

[nebadesmondc]

I created the file(example.diff) as part of the test, but while running the test it gets stuck at this level Done: 99% (115/116, 1 left) (jobs: 1) Does not go pass that. error gotten is :

• diffcessible: internal error, uncaught exception: Unix.Unix_error(Unix.ENODEV, "tcgetattr", "")

[Julow]

That might be because the program never terminate ? :thinking:

[nebadesmondc]

I thought so too, but I haven't figured it out yet. Perhaps I am missing something.

Also, I have a worry. The program normally does not terminate on its own until "q" is entered. This got me confused. Any tips on how to fix this?

[nebadesmondc]

I have made some changes which now permits to accept custom inputs and return the last image. @Julow

[nebadesmondc]

I have made some changes to the PR, please check @panglesd

[nebadesmondc]

Hello @panglesd Thank you for the info, I have done some changes please check

[nebadesmondc]

Hi @Julow,

I've made the changes as requested earlier. However, I wasn't able to resolve the repeated key handler issue. It kept producing unexpected output. Perhaps you could provide some additional details on how I can approach this?

[nebadesmondc]

I have implement the requested changes.

[panglesd]

Nice! Before going further in the review, it would be nice if you could rebase your PR on the newest changes on main!

[nebadesmondc]

Rebase completed

[panglesd]

The rebase did not seem to work properly. The current PR now includes commit that aren't supposed to be included...

[nebadesmondc]

The rebase issue has been rectified. @panglesd

[nebadesmondc]

I have done some changes regarding the issues mentioned above please review

[panglesd]

Two comments!

• You should not create a new renderer each time you want to update the ui, just use the Renderer.update function on your renderer,
• You should not remove a test when you want to add one! While h q is worth testing, so is n, n n, h, ...

[nebadesmondc]

Using Renderer.update is the better option, I have made some changes

[panglesd]

Great!

Now that the PR is finished in terms of functionality, could you do a "cleaning pass" on it? Since many things have been modified, some function might not be needed, some would be better local, ... Try to make the PR the most straightforward: do not use a ref if not needed, avoid recreating a renderer if not needed. Try to modify everything you would ask to modify if you would review the PR!

[nebadesmondc]

I have done some clean up on the PR, Is this okay now?

[nebadesmondc]

Thank you very much @panglesd @Julow for the insights provided on this PR. I have learnt a lot