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 lifecycle script support via --script on env:up #260

Open
wants to merge 8 commits into
base: trunk
Choose a base branch
from

Conversation

Luc45
Copy link
Member

@Luc45 Luc45 commented Mar 14, 2025

This PR adds the ability to run scripts during environment lifecycle:

qit env:up --script ./setup.sh

PS: I'm exposing this just on env:up and not on run:e2e as we already have the Orchestration Process in Custom Tests, and I don't want developers mistakenly using scripts instead of shared/isolated setup/teardown, which could easily break the purpose of the orchestration purpose to manage state in a compatibility test with multiple extensions.

The simplest usage is as above, but you can also run a command directly:

qit env:up --script 'ls -la'

Config File Example

You can also define scripts in a qit.yml|json:

scripts:
  env_started: './setup.sh'
  env_stopped: './teardown.sh'

Testing Instructions

  1. Local script usage

    • Create a shell script, e.g., setup.sh:
      #!/bin/bash
      echo "Hello from setup.sh" > /tmp/test_script.txt
    • qit env:up --script ./setup.sh
    • qit env:exec cat /tmp/test_script.txt]
    • You should see "Hello from setup.sh"
  2. Direct inline command

    • Same as above, but inline: qit env:up --script 'echo "Inline script ran" > /tmp/inline_script.log'
  3. Config-based scripts

    • Create a minimal qit.yml (as shown above).
    • Run:
      qit env:up
    • Confirm that your scripts (setup.sh, teardown.sh) execute on startup or shutdown.
  4. Teardown (Optional)

    • If you use a teardown script (env_stopped), add --script env_stopped=./teardown.sh or define it in qit.yml.
    • Run qit env:down -v and check logs/output to confirm the teardown script runs.

@Luc45 Luc45 requested a review from a team March 14, 2025 01:56
@Luc45 Luc45 self-assigned this Mar 14, 2025
@zhongruige
Copy link
Contributor

This is an awesome addition @Luc45! Quick question--do you think there's any scripts we should try and prevent them running? Granted these are isolated ephemeral environments but not sure if there's anything to be careful of.

@Luc45
Copy link
Member Author

Luc45 commented Mar 14, 2025

do you think there's any scripts we should try and prevent them running?

I don't think so. Everything is dockerized, and any local directory mounted outside the disposable directory is read-only on the host.

Base automatically changed from 25-03/activate-theme-with-parent to trunk March 14, 2025 12:19
@singerb
Copy link
Contributor

singerb commented Mar 14, 2025

I'm exposing this just on env:up and not on run:e2e as we already have the Orchestration Process in Custom Tests, and I don't want developers mistakenly using scripts instead of shared/isolated setup/teardown

Cool change; given this comment though, I'm just curious about the motivation for adding this?

@Luc45
Copy link
Member Author

Luc45 commented Mar 14, 2025

I'm exposing this just on env:up and not on run:e2e as we already have the Orchestration Process in Custom Tests, and I don't want developers mistakenly using scripts instead of shared/isolated setup/teardown

Cool change; given this comment though, I'm just curious about the motivation for adding this?

It's like orchestration but for env:up.

@singerb
Copy link
Contributor

singerb commented Mar 14, 2025

I'm exposing this just on env:up and not on run:e2e as we already have the Orchestration Process in Custom Tests, and I don't want developers mistakenly using scripts instead of shared/isolated setup/teardown

Cool change; given this comment though, I'm just curious about the motivation for adding this?

It's like orchestration but for env:up.

Right, but as you say: custom tests already use orchestration and not this. Did we need this for something, or did vendors request it? That's what I'm curious about.

Copy link
Contributor

@zhongruige zhongruige left a comment

Choose a reason for hiding this comment

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

This is looking good to me so far, I did see @singerb left a question on this one to provide some context around the changes which would be good to have, but otherwise happy to add in my approval.

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.

3 participants