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

Add --start-server arg #87

Merged
merged 6 commits into from
Aug 30, 2023
Merged

Add --start-server arg #87

merged 6 commits into from
Aug 30, 2023

Conversation

sarimarton
Copy link
Collaborator

@sarimarton sarimarton commented Aug 30, 2023

This PR adds a new arg to the command-line interface: --start-server="'<cmd>' <condition>"

The value of the argument is a single string containing a partial command line of the npm package start-server-and-test.

start-server-and-test has the following command line:

start-server-and-test <start server command> <condition> <test command>

where

  • <start server command> is a shell command which starts the app (strictly speaking any command)
  • <condition> is typically a port number which gets polled to monitor when the app is ready
  • <test command> is a shell command which starts the tests

--start-server takes the middle two from this:

--start-server="<start server command> <condition>"

If --start-server is present, Smashtest launches start-server-and-test with the given <start server command> and <condition> args, and with itself as the third (<test commnad>) arg.

Example:

smashtest --headless=false --start-server="'npm start' :5001"

is equal to:

start-server-and-test 'npm start' :5001 'smashtest --headless=false'

As the <start server command> is optional and it defaults to 'npm start' in start-server-and-test, it can be missed here too:

smashtest --headless=false --start-server=:5001

is equal to:

start-server-and-test :5001 'smashtest --headless=false'

Motivation

Starting the app for testing is a common task, and it's not trivial. It cannot be a synchronous shell command, you have to poll for a port to see if the website is available already, and you have to keep track of its process ID and shut down when the running of the test suite is done.

For this reason, it's not suitable to leave it to the user in a *** Before Everything/*** After Everything hook or similar ways.

The popular npm library start-server-and-test solves this problem, but unfortunately, despite its popularity, it's not duly maintained, and it has a bug which only impacts testing processes working with the interactive shell. Smashtest is such a library.

There's a fix for the bug, but who knows when they merge it. In this situation it seems like a good solution to bundle start-server-and-test in the smashtest package.

The bundling works with a package and bundling definition with the patch, so it doesn't increase the repo size.

Of course, the CLI option must be justifiable even after they merge the fix, but I think the option still makes sense. If the fix is merged, we can remove the bundle setup, list start-server-and-test as a dependency in package.json and simply proxy the command to the properly installed start-server-and-test module. Smashtest already incorporates 3rd party libraries, like Sinon, Chai etc., and starting an app server, waiting for a condition, then tearing it down is not a trivial, yet very common task.

@sarimarton sarimarton merged commit df335e3 into master Aug 30, 2023
@sarimarton sarimarton deleted the feat/start-server branch August 30, 2023 12:06
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

Successfully merging this pull request may close these issues.

1 participant