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

💥 Configless operation #108

Merged
merged 19 commits into from
Sep 13, 2024
Merged

💥 Configless operation #108

merged 19 commits into from
Sep 13, 2024

Conversation

randycoulman
Copy link
Owner

💥 BREAKING CHANGE 💥

This PR modifies the way command-line arguments are parsed. If you have any predefined shell or mix aliases using mix test.interactive's --no-watch option or mix test's --exclude option, you will need to update them. If you only pass other mix test options, those will continue to work as-is.

Allow specifying all configuration options from the command line instead of/in addition to application config. This is useful in standard Elixir apps that no longer have a config.exs file by default. It also allows people to override configuration options as needed.

Also adds --help and --version options for convenience.

See the updated documentation for details.

In order to prepare for config-less operation, we need to process the command-line args before starting the main parts of the application.

In order to make that work well, we only start a `DynamicSupervisor` intiially and then start the real supervisor later once we've processed the command-line arguments.
This sets the stage for specifying config options on the command line in addition to settings.
Sets the stage for allowing config settings to be passed on the command line.
- Rename new -> load_from_environment to make what's happening more explicit
- use a raw `%Config{}` struct where env loading isn't needed
- Eliminate duplicate specification of config defaults
- Inline AppConfig.fetch - separate module for this is overkill right now
So far, only the `--clear`/`--no-clear` option is supported; the rest will come in a later commit.

Config options must come first and be separated from `mix test` options by a `--` separator.

The pre-existing `--watch`/`--no-watch` flag can appear in either group for backwards-compatibility, but if it is specified after the `--` separator, a deprecation notice will be printed.

OptionParser has built-in support for recognizing the `--` separator, but I was unable to take full advantage of that so that I could both preserve existing behavior and allow omitting the `--` separator when all arguments are intended for `mix test`.
This includes adding an `--exclude` option that now conflicts with mix test's option of the same name.

I also realized that I've broken backwards-compatibility with the `watch`/`no-watch` option.

I'll decide whether this feature warrants a major version bump and then either fix backwards-compatibility or remove the (not quite correct) handling of the legacy watch/no-watch option.
We've decided to make config-less operation a breaking change. Adjusting to it isn't terribly difficult and simplifies the code and maintenance burden.
Refactors CommandLineParser to return `:ok`/`:error` tuples.
- After calling OptionParser, do a pass to parse special values from strings (regexes and runner modules). This allows more helpful error messages
- Then do a pass to combine `:keep`-style arguments into a single list to simplify config building
Modify the option value parsing logic to return :ok/:error tuples and use reduce_while to collect the results.

There are still two `rescue` clauses, but they're used in order to nicely format error messages since formatting functions aren't exposed by `OptionParser` and `Regex`.
Only show usage information when requested. When there is an error, display a note about using --help instead.
@randycoulman randycoulman merged commit 8256a02 into main Sep 13, 2024
5 checks passed
@randycoulman randycoulman deleted the configless-operation branch September 13, 2024 04:39
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