Turn non-automated tests into Cargo examples #24

Vectornaut · 2024-11-22T04:39:33Z

Vectornaut commented

2024-11-22 04:39:33 +00:00

Some of the Cargo tests on the main branch are designed to print output for human inspection, not to verify computations automatically. The incoming branch turns these tests into Cargo examples. It also makes two organizational changes in pursuit of this goal:

It introduces a dyna3 library target, which the examples use as a dependency. In the future, this target could grow into an officially maintained dyna3 library.
It puts the code for realizing the Irisawa hexlet into a new conditionally compiled engine::irisawa module. This code is shared by a test and an example. Compilation is controlled by the dev feature, which is turned on by default in development mode.

I've verified that printed output of the examples hasn't changed between the head (848f7d6) and base (e917272) of the incoming branch.

Some of the Cargo tests on the main branch are designed to print output for human inspection, not to verify computations automatically. The incoming branch turns these tests into Cargo examples. It also makes two organizational changes in pursuit of this goal: - It introduces a dyna3 library target, which the examples use as a dependency. In the future, this target could grow into an officially maintained dyna3 library. - It puts the code for realizing the Irisawa hexlet into a new conditionally compiled `engine::irisawa` module. This code is shared by a test and an example. Compilation is controlled by the `dev` feature, which is turned on by default in development mode. I've verified that printed output of the examples hasn't changed between the head (848f7d6) and base (e917272) of the incoming branch.

Vectornaut added 5 commits 2024-11-22 04:39:33 +00:00

Turn assertionless tests into Cargo examples 519d0f49df

Streamline Gram matrix setup for Irisawa hexlet e69073a996

Factor out the realization of the Irisawa hexlet de8c662de4

Separate test and example for Irisawa hexlet b23d4a1860

Put shared code in the conditionally compiled `engine::irisawa` module.

Rename the dev feature to reflect its generality 848f7d665b

glen commented

2024-11-25 04:47:09 +00:00

This all looks good, just one minor organizational thing. The file examples/run-examples has no extension, and no hash-bang first line, so it does not self-document how it can be used; nor does the README file or any other documentation mention it, so far as I can see. It is executable, so that is a hint one can try to execute it; and it does work, but as per this SO question, first answer it's not considered best practice to rely on the convention that executable text files will be interpreted by the shell. So please either:

if you'd like this to be an executable file, make it executable (as it already is) with a proper shebang (which it does not have), or
if you'd like to invite it be executed by sh run-examples.sh add the .sh extension to the filename; if it prefers a different shell like bash add the .bash extension to invite bash run-examples.bash. In this case, don't make the file executable, as you wouldn't be inviting that it be executed directly.

One of the above needs to be done. You are also encouraged either in the README to acknowledge app-proto and quickly mention how to run the prototype app (some hints on how to install trunk, then cd app-proto and trunk serve) and how to run the examples, or to start some other appropriate documentation file with this info, likely with a pointer in the README.

Thanks so much! We should be able to merge this shortly.

This all looks good, just one minor organizational thing. The file `examples/run-examples` has no extension, and no hash-bang first line, so it does not self-document how it can be used; nor does the README file or any other documentation mention it, so far as I can see. It _is_ executable, so that is a hint one can try to execute it; and it _does_ work, but as per [this SO question, first answer](https://unix.stackexchange.com/questions/84179) it's not considered best practice to rely on the convention that executable text files will be interpreted by the shell. So please either: * if you'd like this to be an executable file, make it executable (as it already is) with a proper shebang (which it does not have), or * if you'd like to invite it be executed by `sh run-examples.sh` add the `.sh` extension to the filename; if it prefers a different shell like bash add the `.bash` extension to invite `bash run-examples.bash`. In this case, don't make the file executable, as you wouldn't be inviting that it be executed directly. One of the above _needs_ to be done. You are also encouraged either in the README to acknowledge app-proto and quickly mention how to run the prototype app (some hints on how to install trunk, then `cd app-proto` and `trunk serve`) and how to run the examples, or to start some other appropriate documentation file with this info, likely with a pointer in the README. Thanks so much! We should be able to merge this shortly.

👍 1

Vectornaut added 1 commit 2024-11-25 09:39:27 +00:00

Say how to run the prototype, examples, and tests dc5020752b

Vectornaut commented

2024-11-25 09:41:54 +00:00

One of the above needs to be done. You are also encouraged either in the README to acknowledge app-proto and quickly mention how to run the prototype app [...] and how to run the examples, or [...]

Done (in commit dc5020752b).

> One of the above needs to be done. You are also encouraged either in the README to acknowledge app-proto and quickly mention how to run the prototype app [...] and how to run the examples, or [...] Done (in commit dc5020752b).

glen added 1 commit 2024-11-26 00:30:39 +00:00

chore: shebang -> /bin/sh, no bash features used a66a4d17c6

glen merged commit a8e13b8110 into main

2024-11-26 00:32:51 +00:00

glen referenced this issue from a commit

2024-11-26 00:32:52 +00:00

Turn non-automated tests into Cargo examples (#24)

Sign in to join this conversation.

No reviewers