Integration testing: prerequisites #119
Conversation
| else Right stdout | ||
| (_, Always) -> Right stdout | ||
|
|
||
| (ExitSuccess, _) -> Right stdout |
There was a problem hiding this comment.
a lot of unrelated formatting changes in this commit; continuing to do formatting with ormolu piecemeal as I edit files
| Nothing -> pure WalkContinue | ||
| Just _ -> do | ||
| runSimpleStrategy "erlang-rebar3tree" ErlangGroup $ analyze dir | ||
| pure WalkSkipAll |
There was a problem hiding this comment.
This is the fix for the rebar3tree issue
There was a problem hiding this comment.
Why does this fix the issue? Shouldn't we continue walking to find other rebar.config files?
There was a problem hiding this comment.
Specifically, the issue is that dependencies are downloaded into a subdirectory, and those dependencies also contain rebar.configs, e.g., (.rebar/deps isn't accurate here; I forget what the directories are actually called)
myproject/rebar.config
myproject/.rebar/deps/packageA/rebar.config
myproject/.rebar/deps/packageB/rebar.config
..so we end up treating those as direct dependencies.
Worse though, anytime rebar3 tree is called on one of those dependencies, it then downloads another level of dependencies under it, which also contain rebar.configs:
myproject/rebar.config
myproject/.rebar/deps/packageA/rebar.config
myproject/.rebar/deps/packageA/.rebar/deps/packageC/rebar.config
myproject/.rebar/deps/packageB/rebar.config
myproject/.rebar/deps/packageB/.rebar/deps/packageD/rebar.config
and this expands an additional level each time we run analysis
I don't know if the correct option is to ignore rebar.config in subdirectories, or just to blacklist the .rebar/deps (or whatever it's actually called)
There was a problem hiding this comment.
Ah, this makes sense. I like your change for now and we can revert or modify in the future. Thanks for the very detailed response.
| prefix = skipManyTill anySingle (string "install_requires=[") | ||
| entries = between quote quote requirementParser `sepBy` char ',' | ||
| end = char ']' | ||
| prefix = skipManyTill anySingle (symbol "install_requires") *> symbol "=" *> symbol "[" |
There was a problem hiding this comment.
These are the setup.py parser fixes (I didn't know about symbol/lexemes when I first wrote this)
There was a problem hiding this comment.
Do you have an example of what this fixes?
There was a problem hiding this comment.
- whitespace between things, e.g.,
install_requires = \n[ 'foo'] - trailing commas
install_requires=['foo',] - double-quotes used instead of single-quotes
install_requires=["foo"]
| @@ -70,15 +68,15 @@ runStrategy name strategyGroup act = forkTask $ do | |||
| let (bodies, ()) = resultValue result | |||
| in traverse_ (output . toProjectClosure strategyGroup name) bodies -- TODO: warnings | |||
|
|
|||
| type TaskC m = ExecIOC (ReadFSIOC (DiagnosticsC m)) | |||
There was a problem hiding this comment.
Rather than using ExecIOC and ReadFSIOC as part of running the task, we send the constraints upward (see the below additions to HasDiscover)
| Nothing -> pure WalkContinue | ||
| Just _ -> do | ||
| runSimpleStrategy "erlang-rebar3tree" ErlangGroup $ analyze dir | ||
| pure WalkSkipAll |
There was a problem hiding this comment.
Why does this fix the issue? Shouldn't we continue walking to find other rebar.config files?
| prefix = skipManyTill anySingle (string "install_requires=[") | ||
| entries = between quote quote requirementParser `sepBy` char ',' | ||
| end = char ']' | ||
| prefix = skipManyTill anySingle (symbol "install_requires") *> symbol "=" *> symbol "[" |
There was a problem hiding this comment.
Do you have an example of what this fixes?
For integration testing, we want to use a different interpreter of the
Execeffect. As currently written,discoverfunctions userunStrategyto internally interpret theExeceffect, which prevents us from doing so.Instead, these changes force the
Has Exec/Has ReadFSconstraints upward, through thediscover, so we can interpretExecwith a different interpreter in our tests.This also fixes a few issues discovered while building integration tests:
rebar3treewould treat dependencies as top-level projectssetup.pyparser would horribly break if you even looked at it the wrong way (didn't allow double-quotes, was whitespace-sensitive, didn't allow trailing commas in the dependency list)