Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[build] Verify start scripts in ci workflow #41

Open
cbeams opened this issue Jul 20, 2021 · 2 comments
Open

[build] Verify start scripts in ci workflow #41

cbeams opened this issue Jul 20, 2021 · 2 comments

Comments

@cbeams
Copy link
Owner

cbeams commented Jul 20, 2021

There are currently three symlinks in the root of the repository pointing to Gradle-generated start scripts for each of the cli, daemon, and fx apps, respectively:

./bisq -> app/cli/build/install/bisq/bin/bisq
./bisqd -> app/daemon/build/install/bisqd/bin/bisqd
./bisqfx -> app/fx/build/install/bisqfx/bin/bisqfx

I made a change in e0f870c that broke the bisq script, because I updated the name of the underlying main class from BisqCliMain to BisqCLI, but erroneously changed the corresponding mainClass property in build.gradle from BisqCliMain to BisqCli (notice the CLI vs. Cli case difference).

This broke running ./bisq as follows:

$ ./bisq
Error: Could not find or load main class bisq.app.cli.BisqCli
Caused by: java.lang.ClassNotFoundException: bisq.app.cli.BisqCli

But I didn't notice it until now, three days later.

To avoid this kind of breakage in the future, I'd like to verify that these start scripts work as expected during ci, by running them with their --help option in a kind of "smoke test" fashion.

As part of this process, I want to also create symlinks to the Windows .bat variants of the start scripts, and would want the correct variant to be run according to the OS of the runner that the workflow job / step is taking place on.

So there would be a job / step in the workflow that would run ./bisq --help, ./bisqd --help and ./bisqfx --help on *nix and their .bat variants on Windows end verify that each ran successfully by testing that their return status was 0.

At time of writing, only ./bisq and ./bisqfx have --help flags; ./bisqd does not. So a PR that satisfies the above requirements for those two would be sufficient. When ./bisqd grows a --help option it can follow suit.

Aside from preventing breakages, implementing this verification step will also help ensure that these processes remain speedy. Each should take no more than 300–400 ms to run. If we started seeing those steps take longer, we'd know that something is going wrong.

Note that while it would be easier to do verify that the mainClass properties are correct by running a build task, e.g. :app:cli:run --args="--help", but I don't want to do that, because I specifically want to ensure that these scripts and symlinks work as expected. They're an important part of the dev experience being designed here.

@cbeams
Copy link
Owner Author

cbeams commented Jul 20, 2021

@aalmiray, do you think this could be implemented without too much fuss in our workflow? Would like to get your take on the best way to do it, especially considering the conditional nature of which start script needs to be run on *nix vs. Windows. Happy to add this to our scope if you think it's a small effort, e.g. ~1h.

Note that I'm holding off on fixing the capitalization error in the cli mainClass, so that any PR implementing these requirements would fail, showing that the typo exists and proving that the change to the workflow works as expected.

@cbeams
Copy link
Owner Author

cbeams commented Jul 20, 2021

caveat: not sure symlinks to the .bat files would / could work as expected on Windows. Might have to path to them directly instead.

cbeams added a commit that referenced this issue Jul 21, 2021
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

No branches or pull requests

1 participant