Skip to content

Commit 8e7388e

Browse files
teggoticextsoft
authored andcommitted
Improve testability of the Real interpreter
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
1 parent 8abd477 commit 8e7388e

File tree

8 files changed

+346
-173
lines changed

8 files changed

+346
-173
lines changed

.github/workflows/haskell-quality-pipeline.yml

+5-3
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,21 @@ jobs:
2828
- uses: actions/checkout@v3
2929
- uses: actions/cache@v3
3030
name: Cache .ghcup
31+
id: haskell-env-cache
3132
with:
3233
path: ~/.ghcup
3334
key: ${{ runner.os }}-ghcup-global-${{ env.GHCUP_VERSION }}-${{ env.STACK_VERSION }}
3435
restore-keys: |
35-
${{ runner.os }}-ghcup-global-${{ env.GHCUP_VERSION }}
36+
${{ runner.os }}-ghcup-global-${{ env.GHCUP_VERSION }}-
3637
${{ runner.os }}-ghcup-global-
3738
- uses: actions/cache@v3
3839
name: Cache .stack
40+
id: haskell-deps-cache
3941
with:
4042
path: ~/.stack
41-
key: ${{ runner.os }}-stack-global-${{ env.GHC_VERSION }}-${{ hashFiles('stack.yaml') }}-${{ hashFiles('package.yaml') }}-${{ hashFiles('**/*.hs') }}
43+
key: ${{ runner.os }}-stack-global-${{ env.GHC_VERSION }}-${{ hashFiles('stack.yaml') }}-${{ hashFiles('package.yaml') }}
4244
restore-keys: |
43-
${{ runner.os }}-stack-global-${{ env.GHC_VERSION }}
45+
${{ runner.os }}-stack-global-${{ env.GHC_VERSION }}-
4446
- name: Setup haskell enviroment
4547
run: |
4648
export PATH=~/.ghcup/bin:$PATH

elegant-git.cabal

+4
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ library
3030
Elegit.Cli.Command
3131
Elegit.Cli.Parser
3232
Elegit.Git.Action
33+
Elegit.Git.Exec
3334
Elegit.Git.Runner.Real
3435
Elegit.Git.Runner.Simulated
3536
Lib
@@ -46,6 +47,7 @@ library
4647
base >=4.7 && <5
4748
, containers
4849
, dlist
50+
, exceptions
4951
, fmt
5052
, free
5153
, microlens
@@ -79,6 +81,7 @@ executable git-elegant
7981
, containers
8082
, dlist
8183
, elegant-git
84+
, exceptions
8285
, fmt
8386
, free
8487
, microlens
@@ -119,6 +122,7 @@ test-suite elegant-git-test
119122
, containers
120123
, dlist
121124
, elegant-git
125+
, exceptions
122126
, fmt
123127
, free
124128
, hspec

package.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ dependencies:
2424

2525
- universum
2626
- string-qq
27+
- exceptions
2728
- text
2829
- safe-exceptions
2930
- fmt

src/Elegit/Git/Action.hs

+129-32
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
{-# LANGUAGE DeriveFunctor #-}
2-
{-# LANGUAGE DerivingStrategies #-}
3-
{-# LANGUAGE FlexibleContexts #-}
1+
{-# LANGUAGE DeriveFunctor #-}
2+
{-# LANGUAGE DerivingStrategies #-}
3+
{-# LANGUAGE DuplicateRecordFields #-}
4+
{-# LANGUAGE FlexibleContexts #-}
45

56
-----------------------------------------------------------------------------
67
-- |
@@ -35,25 +36,125 @@ import qualified Data.Text as T
3536
import Fmt
3637
import Universum hiding (print)
3738

39+
-- TODO: maybe, cover with tests
40+
class RenderGitCommand c where
41+
renderGC :: c -> Text
42+
43+
data GCurrentBranchData
44+
= GCurrentBranchData
45+
46+
instance RenderGitCommand GCurrentBranchData where
47+
renderGC _ = "rev-parse --abbrev-ref @"
48+
49+
newtype GBranchUpstreamData
50+
= GBranchUpstreamData { branch :: Text }
51+
52+
instance RenderGitCommand GBranchUpstreamData where
53+
renderGC (GBranchUpstreamData branchName) = "rev-parse --abbrev-ref "+|branchName|+"@{upstream}"
54+
55+
data GLogData
56+
= GLogData
57+
{ logType :: LogType
58+
, base :: Text
59+
, target :: Text
60+
}
61+
instance RenderGitCommand GLogData where
62+
renderGC (GLogData lType baseName targetName) = "log "+|logArg|+" "+|baseName|+".."+|targetName|+""
63+
where
64+
logArg :: Text
65+
logArg = case lType of
66+
LogOneLine -> "--oneline"
67+
68+
newtype GStatusData
69+
= GStatusData { statusType :: StatusType }
70+
71+
instance RenderGitCommand GStatusData where
72+
renderGC (GStatusData sType) = "status "+|statusFormat|+""
73+
where
74+
statusFormat :: Text
75+
statusFormat = case sType of
76+
StatusShort -> "--short"
77+
78+
data GStashListData
79+
= GStashListData
80+
instance RenderGitCommand GStashListData where
81+
renderGC _ = "stash list"
82+
83+
data GReadConfigData
84+
= GReadConfigData
85+
{ scope :: ConfigScope
86+
, key :: Text
87+
}
88+
89+
instance RenderGitCommand GReadConfigData where
90+
renderGC (GReadConfigData cScope cName) = "config "+|scopeText|+" --get "+|cName|+""
91+
where
92+
scopeText :: Text
93+
scopeText = case cScope of
94+
LocalConfig -> "--local"
95+
GlobalConfig -> "--global"
96+
AutoConfig -> ""
97+
98+
data GSetConfigData
99+
= GSetConfigData
100+
{ scope :: ConfigScope
101+
, key :: Text
102+
, value :: Text
103+
}
104+
105+
instance RenderGitCommand GSetConfigData where
106+
renderGC (GSetConfigData cScope cName cValue) = "config "+|scopeText|+" "+|cName|+" "+|cValue|+""
107+
where
108+
scopeText :: Text
109+
scopeText = case cScope of
110+
GlobalConfig -> "--global"
111+
LocalConfig -> "--local"
112+
AutoConfig -> "--local"
113+
114+
data GUnsetConfigData
115+
= GUnsetConfigData
116+
{ scope :: ConfigScope
117+
, key :: Text
118+
}
119+
120+
instance RenderGitCommand GUnsetConfigData where
121+
renderGC (GUnsetConfigData cScope cName) = "config "+|scopeText|+" --unset "+|cName|+""
122+
where
123+
scopeText :: Text
124+
scopeText = case cScope of
125+
GlobalConfig -> "--global"
126+
LocalConfig -> "--local"
127+
AutoConfig -> "--local"
128+
129+
newtype GAliasesToRemoveData
130+
= GAliasesToRemoveData { scope :: ConfigScope }
131+
132+
instance RenderGitCommand GAliasesToRemoveData where
133+
renderGC (GAliasesToRemoveData cScope) = "config "+|scopeText|+" --name-only --get-regexp \"^alias.\" \"^elegant ([-a-z]+)$\""
134+
where
135+
scopeText :: Text
136+
scopeText = case cScope of
137+
GlobalConfig -> "--global"
138+
LocalConfig -> "--local"
139+
AutoConfig -> ""
140+
38141
-- | The declaration of all posible actions we can do in the git action.
39142
--
40143
-- This describes the data of the action, and whether it can return any value
41144
-- for further computations.
42145
--
43146
-- We can use records later to better comunicate the purpose of each field by
44147
-- providing a name.
45-
--
46-
-- TODO: Use records
47148
data GitF a
48-
= CurrentBranch (Text -> a)
49-
| BranchUpstream Text (Maybe Text -> a)
50-
| Log LogType Text Text ([Text] -> a)
51-
| Status StatusType ([Text] -> a)
52-
| StashList ([Text] -> a)
53-
| ReadConfig ConfigScope Text (Maybe Text -> a)
54-
| AliasesToRemove ConfigScope (Maybe (NonEmpty Text) -> a)
55-
| SetConfig ConfigScope Text Text a
56-
| UnsetConfig ConfigScope Text a
149+
= CurrentBranch GCurrentBranchData (Text -> a)
150+
| BranchUpstream GBranchUpstreamData (Maybe Text -> a)
151+
| Log GLogData ([Text] -> a)
152+
| Status GStatusData ([Text] -> a)
153+
| StashList GStashListData ([Text] -> a)
154+
| ReadConfig GReadConfigData (Maybe Text -> a)
155+
| AliasesToRemove GAliasesToRemoveData (Maybe (NonEmpty Text) -> a)
156+
| SetConfig GSetConfigData a
157+
| UnsetConfig GUnsetConfigData a
57158
| Prompt Text (Maybe Text) (Text -> a)
58159
| FormatInfo Text (Text -> a)
59160
| FormatCommand Text (Text -> a)
@@ -101,34 +202,34 @@ type FreeGit t = F GitF t
101202
-- * Otherwise just use `id` function.
102203

103204
status :: MonadFree GitF m => StatusType -> m [Text]
104-
status sType = liftF $ Status sType id
205+
status sType = liftF $ Status (GStatusData sType) id
105206

106207
log :: MonadFree GitF m => LogType -> Text -> Text -> m [Text]
107-
log lType lBase lTarget = liftF $ Log lType lBase lTarget id
208+
log lType lBase lTarget = liftF $ Log (GLogData lType lBase lTarget) id
108209

109210
stashList :: MonadFree GitF m => m [Text]
110-
stashList = liftF $ StashList id
211+
stashList = liftF $ StashList GStashListData id
111212

112213
currentBranch :: MonadFree GitF m => m Text
113-
currentBranch = liftF $ CurrentBranch id
214+
currentBranch = liftF $ CurrentBranch GCurrentBranchData id
114215

115216
branchUpstream :: MonadFree GitF m => Text -> m (Maybe Text)
116-
branchUpstream bName = liftF $ BranchUpstream bName id
217+
branchUpstream bName = liftF $ BranchUpstream (GBranchUpstreamData bName) id
117218

118219
readConfig :: MonadFree GitF m => ConfigScope -> Text -> m (Maybe Text)
119-
readConfig cScope cName = liftF $ ReadConfig cScope cName id
220+
readConfig cScope cName = liftF $ ReadConfig (GReadConfigData cScope cName) id
120221

121-
-- TODO: Check if it's better or even possible to get all configurations and filter then ourself.
222+
-- TODO: Check if it's better or even possible to get all configurations and filter them ourself.
122223
-- This would improve testability of this, as now we rely on the fact that we make a correct cli call
123224
-- to find config with regex in the `Real.hs`.
124225
aliasesToRemove :: MonadFree GitF m => ConfigScope -> m (Maybe (NonEmpty Text))
125-
aliasesToRemove cScope = liftF $ AliasesToRemove cScope id
226+
aliasesToRemove cScope = liftF $ AliasesToRemove (GAliasesToRemoveData cScope) id
126227

127228
setConfig :: MonadFree GitF m => ConfigScope -> Text -> Text -> m ()
128-
setConfig cScope cName cValue = liftF $ SetConfig cScope cName cValue ()
229+
setConfig cScope cName cValue = liftF $ SetConfig (GSetConfigData cScope cName cValue) ()
129230

130231
unsetConfig :: MonadFree GitF m => ConfigScope -> Text -> m ()
131-
unsetConfig cScope cName = liftF $ UnsetConfig cScope cName ()
232+
unsetConfig cScope cName = liftF $ UnsetConfig (GUnsetConfigData cScope cName) ()
132233

133234
promptDefault :: MonadFree GitF m => Text -> Maybe Text -> m Text
134235
promptDefault pText pDefault = liftF $ Prompt pText pDefault id
@@ -144,22 +245,18 @@ print content = liftF $ PrintText content ()
144245

145246
-- Derived actions
146247

147-
configScopeText :: ConfigScope -> Text
148-
configScopeText LocalConfig = "--local"
149-
configScopeText GlobalConfig = "--global"
150-
configScopeText AutoConfig = ""
248+
formatGitCommand :: (RenderGitCommand gc, MonadFree GitF m) => gc -> m Text
249+
formatGitCommand gc = formatCommand ("git "+|renderGC gc|+"")
151250

152251
setConfigVerbose :: MonadFree GitF m => ConfigScope -> Text -> Text -> m ()
153252
setConfigVerbose cScope cName cValue = do
154253
setConfig cScope cName cValue
155-
-- TODO: Generate these messages only in a single place and reuse in `Real.hs`
156-
print =<< formatCommand ("git config "+|configScopeText cScope|+" "+|cName|+" "+|cValue|+"")
254+
print =<< formatGitCommand (GSetConfigData cScope cName cValue)
157255

158256
unsetConfigVerbose :: MonadFree GitF m => ConfigScope -> Text -> m ()
159257
unsetConfigVerbose cScope cName = do
160258
unsetConfig cScope cName
161-
-- TODO: Generate these messages only in a single place and reuse in `Real.hs`
162-
print =<< formatCommand ("git config "+|configScopeText cScope|+" --unset "+|cName|+"")
259+
print =<< formatGitCommand (GUnsetConfigData cScope cName)
163260

164261
freshestDefaultBranch :: MonadFree GitF m => m Text
165262
freshestDefaultBranch = do

0 commit comments

Comments
 (0)