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

LUAFDN-784 Try to download all tools before erroring #62

Merged
merged 24 commits into from
Jan 23, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/config.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{collections::HashMap, env, fmt};
use std::{collections::BTreeMap, env, fmt};

use semver::VersionReq;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -69,13 +69,13 @@ impl fmt::Display for ToolSpec {

#[derive(Debug, Serialize, Deserialize)]
pub struct ConfigFile {
pub tools: HashMap<String, ToolSpec>,
pub tools: BTreeMap<String, ToolSpec>,
}

impl ConfigFile {
pub fn new() -> Self {
Self {
tools: HashMap::new(),
tools: BTreeMap::new(),
}
}

Expand Down
6 changes: 6 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ pub enum ForemanError {
current_path: PathBuf,
config_file: ConfigFile,
},
ToolsNotDownloaded {
tools: Vec<String>,
},
}

impl ForemanError {
Expand Down Expand Up @@ -302,6 +305,9 @@ impl fmt::Display for ForemanError {
current_path.display(),
config_file,
),
Self::ToolsNotDownloaded { tools } => {
write!(f, "The following tools were not installed:\n{:#?}", tools)
}
}
}
}
Expand Down
39 changes: 32 additions & 7 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,18 +189,39 @@ fn actual_main(paths: ForemanPaths) -> ForemanResult<()> {
.init();
}

match options.subcommand {
let subcommand_result: ForemanResult<()> = match options.subcommand {
hmallen99 marked this conversation as resolved.
Show resolved Hide resolved
Subcommand::Install => {
let config = ConfigFile::aggregate(&paths)?;

log::trace!("Installing from gathered config: {:#?}", config);

let mut cache = ToolCache::load(&paths)?;

for (tool_alias, tool_spec) in &config.tools {
let providers = ToolProvider::new(&paths);
cache.download_if_necessary(tool_spec, &providers)?;
add_self_alias(tool_alias, &paths.bin_dir())?;
let providers = ToolProvider::new(&paths);

let tools_not_downloaded: Vec<String> = config
.tools
.iter()
.filter_map(|(tool_alias, tool_spec)| {
cache
.download_if_necessary(tool_spec, &providers)
.and_then(|_| add_self_alias(tool_alias, &paths.bin_dir()))
.err()
.map(|err| {
log::error!(
"The following error occurred while trying to download tool \"{}\":\n{}",
tool_alias,
err
);
tool_alias.to_string()
})
})
.collect();

if !tools_not_downloaded.is_empty() {
return Err(ForemanError::ToolsNotDownloaded {
tools: tools_not_downloaded,
});
}

if config.tools.is_empty() {
Expand All @@ -213,6 +234,7 @@ fn actual_main(paths: ForemanPaths) -> ForemanResult<()> {
paths.user_config().display()
);
}
Ok(())
}
Subcommand::List => {
println!("Installed tools:");
Expand All @@ -226,6 +248,7 @@ fn actual_main(paths: ForemanPaths) -> ForemanResult<()> {
println!(" - {}", version);
}
}
Ok(())
}
Subcommand::GitHubAuth(subcommand) => {
let token = prompt_auth_token(
Expand All @@ -237,6 +260,7 @@ fn actual_main(paths: ForemanPaths) -> ForemanResult<()> {
AuthStore::set_github_token(&paths.auth_store(), &token)?;

println!("GitHub auth saved successfully.");
Ok(())
}
Subcommand::GitLabAuth(subcommand) => {
let token = prompt_auth_token(
Expand All @@ -248,10 +272,11 @@ fn actual_main(paths: ForemanPaths) -> ForemanResult<()> {
AuthStore::set_gitlab_token(&paths.auth_store(), &token)?;

println!("GitLab auth saved successfully.");
Ok(())
}
}
};

Ok(())
subcommand_result
}

fn prompt_auth_token(
Expand Down
18 changes: 18 additions & 0 deletions tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,21 @@ stylua = { github = "JohnnyMorganz/StyLua", version = "0.11.3" }
);
context.snapshot_command("install_invalid_auth_configuration");
}

#[test]
fn snapshot_install_all_tools_before_failing() {
let mut context = TestContext::foreman().arg("install");
let config_path = context.path_from_working_directory("foreman.toml");
write_file(
&config_path,
r#"
[tools]
stylua = { github = "JohnnyMorganz/StyLua", version = "0.11.3" }
not-a-real-tool = { github = "Roblox/VeryFakeRepository", version = "0.1.0" }
badly-formatted-tool = { github = "Roblox/", version = "0.2.0" }
selene = { source = "Kampfkarren/selene", version = "=0.22.0" }
Copy link
Contributor Author

@hmallen99 hmallen99 Dec 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test ends up actually downloading these tools. Is there any way to mock downloading stylua and selene? I want to have a "successful" download alongside the failed ones.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is possible yes but you will have to refactor the code to inject a custom ToolProvider implementation. This is probably easier to do within the src code. Tests in tests are considered at least integration level and so they cannot access private members. So if you refactor the code a little, you can write a unit test in main.rs that injects a memory only tool provider

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll just add an e2e test with the actual downloads and just test out the download failure here. I don't want the scope of this PR to get too large, so I might just file an issue to inject a mocked tool provider if there isn't one already.

"#,
);

context.snapshot_command("install_all_tools_before_failing");
}
27 changes: 27 additions & 0 deletions tests/snapshots/install_all_tools_before_failing.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
source: tests/cli.rs
assertion_line: 101
expression: content
---
[INFO ] Downloading github.com/Roblox/@^0.2.0
[ERROR] The following error occurred while trying to download tool "badly-formatted-tool":
unexpected response body: invalid type: map, expected a sequence at line 1 column 0
Request from `https://api.github.com/repos/Roblox//releases`

Received body:
{"message":"Not Found","documentation_url":"https://docs.github.com/rest"}
[INFO ] Downloading github.com/Roblox/VeryFakeRepository@^0.1.0
[ERROR] The following error occurred while trying to download tool "not-a-real-tool":
unexpected response body: invalid type: map, expected a sequence at line 1 column 0
Request from `https://api.github.com/repos/Roblox/VeryFakeRepository/releases`

Received body:
{"message":"Not Found","documentation_url":"https://docs.github.com/rest/reference/repos#list-releases"}
[INFO ] Downloading github.com/Kampfkarren/selene@=0.22.0
[INFO ] Downloading github.com/JohnnyMorganz/StyLua@^0.11.3
The following tools were not installed:
[
"badly-formatted-tool",
"not-a-real-tool",
]

36 changes: 20 additions & 16 deletions tests/snapshots/install_invalid_auth_configuration.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,26 @@
source: tests/cli.rs
assertion_line: 101
expression: content

---
[INFO ] Downloading github.com/JohnnyMorganz/StyLua@^0.11.3
unable to parse Foreman authentication file (at {{FOREMAN_HOME}}auth.toml): expected an equals, found eof at line 1 column 8

A Foreman authentication file looks like this:

# For authenticating with github.com, put a personal access token here under the
# `github` key. This is useful if you hit GitHub API rate limits or if you need
# to access private tools.

github = "YOUR_TOKEN_HERE"

# For authenticating with GitLab.com, put a personal access token here under the
# `gitlab` key. This is useful if you hit GitLab API rate limits or if you need
# to access private tools.

gitlab = "YOUR_TOKEN_HERE"
[ERROR] The following error occurred while trying to download tool "stylua":
unable to parse Foreman authentication file (at {{FOREMAN_HOME}}auth.toml): expected an equals, found eof at line 1 column 8

A Foreman authentication file looks like this:

# For authenticating with github.com, put a personal access token here under the
# `github` key. This is useful if you hit GitHub API rate limits or if you need
# to access private tools.

github = "YOUR_TOKEN_HERE"

# For authenticating with GitLab.com, put a personal access token here under the
# `gitlab` key. This is useful if you hit GitLab API rate limits or if you need
# to access private tools.

gitlab = "YOUR_TOKEN_HERE"
The following tools were not installed:
[
"stylua",
]