Integration tests #338
Integration tests #338
Conversation
make route formatting more consistent
| # --test '*' means only run the integration tests | ||
| # https://github.com/rust-lang/cargo/issues/8396#issuecomment-713126649 | ||
| # --nocapture is used so that we see the commands being executed interleaved within the logged info | ||
| GIT_DAEMON_BASE_PATH_TRACKER="$git_daemon_base_path_tracker" cargo test --test '*' -- --nocapture |
There was a problem hiding this comment.
I'd really recommend just using a proper library like https://docs.rs/git2/0.13.23/git2/ over worrying about all of this stuff, then we can simply just run cargo test maybe
There was a problem hiding this comment.
git-daemon is tricky in the sense that the daemon's process is not killed when the main process is, so I resort to this kind of measure. The library you posted does not appear to have anything related to the daemon in any case.
If you'd like to pursue this option then please open a new issue for that.
| @@ -43,6 +46,12 @@ impl MainConfig { | |||
| }) | |||
| .unwrap_or(false); | |||
|
|
|||
| let github_api_url = "https://api.github.com".to_owned(); | |||
There was a problem hiding this comment.
Worth having this as an env var? Or setting up env vars for tests?
There was a problem hiding this comment.
For tests, each test will have its own Github mock API and so setting up an environment variable is not as clean or as reasonable as storing it in the state.
For the application, we do not have a use-case for a custom Github API and so I'd rather not introduce a new environment variable.
| } | ||
|
|
||
| pub fn initialize_repository(repo_dir: &Path, initial_branch: &str) { | ||
| exec::<&str, PathBuf>( |
There was a problem hiding this comment.
I'm not opposed to using a library, but it definitely will not be done in this MR. Feel free to open a new issue for that.
Food for thought: even though using that library might subjectively might appear to be cleaner, I infer it would increase complexity because, while we can reason about git commands which we use on a daily basis, by introducing that library all maintainers would have to learn about how it works in order to deal with this function (which is merely a test helper, not actually application code). From my point-of-view the tradeoff is not worth it given that this exec interface which works well enough (including exit code, stderr visibility, etc...) for any commands we want to execute, not only git ones.
Adds an integration test and runs it on CI.
This PR includes a single simple integration test because its main purpose is to introduce the ability to running integration tests in the first place, since we don't have any. In the future, more integration tests can be added upon the code introduced here.
The original branch contained a new feature which I've not included here to reduce the scope of this PR and also due to time constraints.
closes #251