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

Improve testability of the Real interpreter #300

Merged
merged 2 commits into from
Mar 15, 2023
Merged

Improve testability of the Real interpreter #300

merged 2 commits into from
Mar 15, 2023

Conversation

teggotic
Copy link
Contributor

@teggotic teggotic commented Feb 15, 2023

There is an issue in how we test actions. Fundamentally, we rely on the fact that the Real runner is generating git calls correctly. This could be neglected in theory, but there is another issue, which forces us to generate "textual" representation of what our command is doing, in multiple places, which leads to code repetition.
As a solution to this, I've created 1 data type for every type of the command we can execute, and implemeted the way to render it to the "textual" representation.
This could be used by Real runner directly, but this still suffers from a different issue. We give Real runner to much access, by adding MonadIO to the context.
There is a solution however. We can create a custom Monad which can provide all needed functions for Real to proceed. Going this way, we can hide how we "render" and "execute" git calls from the Real runner, which implies that it has no way to run arbitrary commands at all.

Values to git config should be quoted to work properly, otherwise they
(specifically aliases) would be set as multi-value configs.

However, if we just blindly change this to be quoted values, we would
split them into separate arguments for a proc, which would lead to the
same issue.
To improve this, we just need to use [] instead of Text for command
args.

Note: I've switched to "raw strings" ([s|quoted "string"|]) in
AcquireRepositorySpec.hs to remove escaping of double quotes.

#299

The contribution:

  • updates all affected documentation
  • provides commits messages which comply with the CONTRIBUTING.md > Committing the changes rules
  • updates the completion scripts if requires
  • complies with all requirements from README.md > Hands-on development notes

@bees-hive/elegant-git-maintainers, please review.

Base automatically changed from 297 to main February 19, 2023 02:03
@extsoft
Copy link
Contributor

extsoft commented Feb 19, 2023

@teggotic please resolve conflicts here

There is an issue in how we test actions. Fundamentally, we rely on the
fact that the `Real` runner is generating `git` calls correctly. This
could be neglected in theory, but there is another issue, which forces
us to generate "textual" representation of what our command is doing, in
multiple places, which leads to code repetition.
As a solution to this, I've created 1 data type for every type of the
command we can execute, and implemeted the way to render it to the
"textual" representation.
This could be used by `Real` runner directly, but this still suffers
from a different issue. We give `Real` runner to much access, by adding
`MonadIO` to the context.
There is a solution however. We can create a custom `Monad` which can
provide all needed functions for `Real` to proceed. Going this way, we
can hide how we "render" and "execute" git calls from the `Real` runner,
which implies that it has no way to run arbitrary commands at all.

#299
@teggotic
Copy link
Contributor Author

@teggotic please resolve conflicts here

Sure, @extsoft please check

Values to `git config` should be quoted to work properly, otherwise they
(specifically aliases) would be set as multi-value configs.

However, if we just blindly change this to be quoted values, we would
split them into separate arguments for a proc, which would lead to the
same issue.
To improve this, we just need to use `[]` instead of `Text` for command
args.

Note: I've switched to "raw strings" (`[s|raw string|]`) in
`AcquireRepositorySpec.hs` to remove escaping of double quotes.

#299
@extsoft extsoft merged commit e005428 into main Mar 15, 2023
@extsoft extsoft deleted the 299 branch March 15, 2023 02:26
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.

None yet

2 participants