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

feat(test): New UI implementation for ie test & refactor various parts of the engine #198

Merged
merged 28 commits into from
Jul 2, 2024

Conversation

vmarcella
Copy link
Member

@vmarcella vmarcella commented Jun 3, 2024

This PR:

  • Updates ie test to use bubbletea for rendering it's output, similar to ie interactive and soon ie execute.
  • Refactors ie interactive and ie test implementations to exist in their own folders inside of engine (Which will be renamed to ie in the future).
  • Refactors code commonly used across execute, interactive, and test modes into a new package found in internal/engine/common.
  • Decoupled models from requiring a pointer to Engine as a parameter in preparation for when it will be deprecated in the future.
  • Reimplements shells.ExecuteCodeBlock to be a variable alias of shells.executeCodeBlockImpl. This allows for us to easily mock the the ExecuteCodeBlock implementation easily in tests. This allows us to do things like track calls to commands, record what commands were failed, mimic failures from executing commands, etc without having to actually execute commands. This is particularly useful in cases where we want to test the behavior of executing an Azure CLI command without actually executing the command itself.
  • Fixed an issue with the regex used to locate resource groups inside of command outputs and added tests to ensure that it works.
  • Adds a new environment github-action, specifically for running ie test inside of github actions runners. If this flag is not supplied, ie test will crash due to bubbletea attempting to open tty when there are no ttys available. (See Not a tty actions/runner#241 for more information on GH actions not providing a TTY).

@vmarcella vmarcella temporarily deployed to ScenarioTesting June 3, 2024 20:15 — with GitHub Actions Inactive
@vmarcella vmarcella temporarily deployed to ScenarioTesting June 3, 2024 23:34 — with GitHub Actions Inactive
@mbifeld
Copy link
Member

mbifeld commented Jun 12, 2024

Fixes #66

internal/engine/engine.go Outdated Show resolved Hide resolved
…t in github actions, update logic for deleting resource groups after execution, and add more tests to test-mode.
@vmarcella vmarcella temporarily deployed to ScenarioTesting June 17, 2024 23:26 — with GitHub Actions Inactive
@vmarcella vmarcella temporarily deployed to ScenarioTesting June 17, 2024 23:28 — with GitHub Actions Inactive
…atch the resource group name, update ExecuteBashCommand to be a variable exported from shells/bash.go for easier mocking, and update model tests.
@vmarcella vmarcella temporarily deployed to ScenarioTesting June 18, 2024 22:35 — with GitHub Actions Inactive
… the FailedCommandMessage to be part of the error output and update CompareCommandOutputs to return better error messages to describe what happened.
@vmarcella vmarcella temporarily deployed to ScenarioTesting June 19, 2024 17:56 — with GitHub Actions Inactive
@vmarcella vmarcella temporarily deployed to ScenarioTesting June 21, 2024 23:12 — with GitHub Actions Inactive
@vmarcella vmarcella temporarily deployed to ScenarioTesting June 21, 2024 23:14 — with GitHub Actions Inactive
…e can't assume that all cis will not have ttys.
@vmarcella vmarcella temporarily deployed to ScenarioTesting June 25, 2024 20:37 — with GitHub Actions Inactive
@vmarcella vmarcella changed the title Update ie test UI implementation & refactor various parts of the engine feat(test): New UI implementation for ie test & refactor various parts of the engine Jun 25, 2024
Copy link
Member

@mbifeld mbifeld left a comment

Choose a reason for hiding this comment

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

LGTM. I'm curious if you could share what the crash behavior is when ie test is ran in GitHub actions without the needed flag. How should the user know they are missing the flag?

@vmarcella vmarcella added this pull request to the merge queue Jul 2, 2024
Merged via the queue into main with commit 37862b0 Jul 2, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants