Skip to content

Add a few basic tests for the compiled executable - #10

Merged
pyrmont merged 6 commits into
pyrmont:masterfrom
engenforge:test-executable
Oct 15, 2025
Merged

Add a few basic tests for the compiled executable#10
pyrmont merged 6 commits into
pyrmont:masterfrom
engenforge:test-executable

Conversation

@engenforge

Copy link
Copy Markdown
Contributor

Rather than creating a shell script to test the executable I added some test cases using the existing testament test runner in the test folder. The tests fail if the user does not first compile/build the predoc executable. I wasn't sure if there was an easy way to add a dependency similar to a Makefile that would compile predoc if it didn't exist when running the test system.

@pyrmont pyrmont self-assigned this Oct 15, 2025
@pyrmont pyrmont added the tooling Update to tooling label Oct 15, 2025

@pyrmont pyrmont left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you so much for going to the effort of making the test file (and using things like Testament!). I'm sorry it took so long for me to review.

I did refactor it a bit so that it would work without the user needing to have built the executable in advance. Let me know if it looks all right from your side!

@pyrmont pyrmont linked an issue Oct 15, 2025 that may be closed by this pull request
@engenforge

engenforge commented Oct 15, 2025

Copy link
Copy Markdown
Contributor Author

This looks great and I like your changes to build the executable automatically. There are two remaining concerns or questions that I have:

  1. It looks like there is a new hard dependency on jeep now?
  2. The executable that the test runs on (tmp/predoc) is different from the default executable build ./predoc

For the first item, prior to this the executable could be built and testing could be completed with only janet as a dependency. From a packaging perspective I would need to create a package for jeep, and make that a build dependency for predoc. If that's the intent I can do that, but there is a certain appeal to having minimal dependencies. IMHO it would be great from a packaging perspective if predoc could continue to be built and tested with only janet as a dependency, while also having the codebase structured to work well with developers who choose to use jeep. My assumption is that this could be done fairly easily by changing one line as mentioned above?

The second item is starting to get into semantics, but ideally from a packaging perspective the executable that is installed should be the same one that is tested in test/executable.janet. In typical packaging scenario's, there is a build stage, which is followed by a testing stage, which is followed by an install stage. So in this current scenario ./predoc would be built first, and then tmp/predoc would be built, tested, and deleted, and then ./predoc would be installed assuming the tests passed. Theoretically ./predoc and tmp/predoc should be binary identical, but there is a risk that they might not be. Also it's less efficient for os build systems in that they have to build predoc twice. Can we use the same executable ./predoc, and in the defer function in test/executable.janet only build it if it doesn't already exist?

If you agree with one or both of these I can work on making the changes. I wanted to check with you first though before working on anything. Thanks!

@pyrmont

pyrmont commented Oct 15, 2025

Copy link
Copy Markdown
Owner
  1. It looks like there is a new hard dependency on jeep now?

I confess my brain broke and I completely forgot that I had all the files in ./bundle to allow the predoc binary to be built with only the janet binary. I’ve removed the Jeep requirement from both the ./test/executable.janet file and the test GitHub Workflow.

  1. The executable that the test runs on (tmp/predoc) is different from the default executable build ./predoc

I’ve followed your suggestion and now check whether the binary is in the bundle root. If it is, the code copies the file into ./tmp and uses that. If it isn’t, it creates the quickbin and moves it to ./tmp.

Apologies for forgetting how my own project works! :D

@engenforge

engenforge commented Oct 15, 2025

Copy link
Copy Markdown
Contributor Author

Looks great! Thank you very much.

By the way, could you create a new release after this merges?

@pyrmont
pyrmont merged commit 8cb0c17 into pyrmont:master Oct 15, 2025
1 check passed
@pyrmont

pyrmont commented Oct 15, 2025

Copy link
Copy Markdown
Owner

Thanks for taking the time to write the tests! It’s great to have this as part of the suite now :)

@pyrmont

pyrmont commented Oct 15, 2025

Copy link
Copy Markdown
Owner

By the way, could you create a new release after this merges?

Done!

@engenforge
engenforge deleted the test-executable branch October 15, 2025 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tooling Update to tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Creating tests for the predoc executable file

2 participants