Skip to content

Improve testability of the Real interpreter#300

Merged
extsoft merged 2 commits into
mainfrom
299
Mar 15, 2023
Merged

Improve testability of the Real interpreter#300
extsoft merged 2 commits into
mainfrom
299

Conversation

@teggotic

@teggotic teggotic commented Feb 15, 2023

Copy link
Copy Markdown
Contributor

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.

@teggotic teggotic requested a review from extsoft February 15, 2023 00:52
@teggotic teggotic force-pushed the 299 branch 2 times, most recently from 54e9c99 to dfaf6c8 Compare February 15, 2023 01:20
Base automatically changed from 297 to main February 19, 2023 02:03
@extsoft

extsoft commented Feb 19, 2023

Copy link
Copy Markdown
Owner

@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
Copy Markdown
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.

2 participants