Skip to content

Commit 99a219c

Browse files
fix tests (#1575)
Co-authored-by: SupperZum <[email protected]>
1 parent 38bcb56 commit 99a219c

File tree

12 files changed

+171
-51
lines changed

12 files changed

+171
-51
lines changed

module/move/willbe/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ enabled = [
4343
"wca/enabled",
4444
"pth/enabled",
4545
"process_tools/enabled",
46-
"derive_tools/enabled",
4746
"data_type/enabled",
4847
"collection_tools/enabled",
4948
"macro_tools/enabled",
@@ -81,6 +80,7 @@ serde = "1.0" # for CargoMetadata::Package
8180
parse-display = "0.9" # need because derive_tools don't reexport this correctly
8281
walkdir = "2.3"
8382
rustdoc-md = "0.1.0"
83+
assert_fs = "1.1.3"
8484

8585
## internal
8686
# qqq : optimize features
@@ -93,7 +93,7 @@ mod_interface = { workspace = true, features = [ "default" ] }
9393
wca = { workspace = true, features = [ "default" ] }
9494
pth = { workspace = true, features = [ "default", "path_utf8" ] }
9595
process_tools = { workspace = true, features = [ "default" ] }
96-
derive_tools = { workspace = true, features = [ "derive_display", "derive_from_str", "derive_deref", "derive_from", "derive_as_ref" ] }
96+
derive_tools = { workspace = true, features = [ "default" ] } # derive_tools is a basic dependency required for compilation.
9797
data_type = { workspace = true, features = [ "either" ] }
9898
collection_tools = { workspace = true, features = [ "collection_constructors", "collection_into_constructors" ] }
9999
macro_tools = { workspace = true, features = [ "default" ] }

module/move/willbe/src/action/list.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,27 @@ mod private
438438
///
439439
/// - `Result<ListReport, (ListReport, Error)>` - A result containing the list report if successful,
440440
/// or a tuple containing the list report and error if not successful.
441+
/// # Errors
442+
///
443+
/// Returns an error if it fails to read the workspace manifest, parse dependencies,
444+
/// or if a dependency cycle is detected in topological sort mode.
445+
///
446+
/// # Panics
447+
///
448+
/// The function may panic if it encounters a package version that cannot be parsed
449+
/// into a valid `semver::VersionReq`. This can happen with malformed `Cargo.toml` files.
450+
///
451+
/// # Errors
452+
///
453+
/// Returns an error if it fails to read the workspace manifest, parse dependencies,
454+
/// or if a dependency cycle is detected in topological sort mode.
455+
///
456+
/// # Panics
457+
///
458+
/// The function may panic if it encounters a package version that cannot be parsed
459+
/// into a valid `semver::VersionReq`. This can happen with malformed `Cargo.toml` files.
460+
///
461+
#[ allow( clippy::too_many_lines ) ]
441462
#[ cfg_attr( feature = "tracing", tracing::instrument ) ]
442463
pub fn list_all( args : ListOptions )
443464
-> ResultWithReport< ListReport, error::untyped::Error > // qqq : should be specific error

module/move/willbe/src/action/publish.rs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,19 @@ mod private
115115
///
116116
/// # Returns
117117
/// A Result containing a `PublishPlan` if successful, or an `Error` otherwise.
118+
///
119+
/// # Errors
120+
///
121+
/// Returns an error if it fails to find packages, read the workspace, or create a temporary directory.
122+
///
123+
/// # Panics
124+
///
125+
/// Panics if `patterns` is not empty but resolving the first path to a workspace fails,
126+
/// or if toposort fails on the dependency graph.
118127
#[ cfg_attr( feature = "tracing", tracing::instrument ) ]
119128
pub fn publish_plan
120129
(
121-
patterns : Vec< String >,
130+
patterns : &[ String ],
122131
channel : channel::Channel,
123132
dry : bool,
124133
temp : bool
@@ -128,7 +137,7 @@ mod private
128137
{
129138
let mut paths = collection::HashSet::new();
130139
// find all packages by specified folders
131-
for pattern in &patterns
140+
for pattern in patterns
132141
{
133142
let current_path = AbsolutePath::try_from
134143
(
@@ -175,7 +184,7 @@ mod private
175184
&graph,
176185
&packages_to_publish[ .. ]
177186
);
178-
let tmp = subgraph_wanted
187+
let tmp_subgraph = subgraph_wanted
179188
.map
180189
(
181190
| _, n |
@@ -212,8 +221,9 @@ mod private
212221

213222
let subgraph = graph::remove_not_required_to_publish
214223
(
224+
&workspace,
215225
&package_map,
216-
&tmp,
226+
&tmp_subgraph,
217227
&packages_to_publish,
218228
dir.clone(),
219229
)?;
@@ -246,7 +256,14 @@ mod private
246256
///
247257
/// Publish packages.
248258
///
249-
259+
/// # Errors
260+
///
261+
/// Returns an error if any of the publishing steps fail or if cleanup of temporary directories fails.
262+
///
263+
/// # Panics
264+
///
265+
/// Panics if the report for a successfully published package is missing expected information.
266+
#[ allow( clippy::result_large_err ) ]
250267
#[ cfg_attr( feature = "tracing", tracing::instrument ) ]
251268
pub fn publish( plan : publish::PublishPlan )
252269
->

module/move/willbe/src/action/publish_diff.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,25 @@ mod private
9999
}
100100

101101
/// Return the differences between a local and remote package versions.
102+
///
103+
/// # Errors
104+
///
105+
/// Returns an error if there's an issue with path conversion, packing the local crate,
106+
/// or if the internal `list` action returns an unexpected format.
107+
///
108+
/// # Panics
109+
///
110+
/// This function may panic if the internal `list_all` action fails, if it's unable to download
111+
/// the package from crates.io, or if a dependency tree walk encounters an unexpected structure.
102112
#[ cfg_attr( feature = "tracing", tracing::instrument ) ]
103113
pub fn publish_diff( o : PublishDiffOptions ) -> Result< PublishDiffReport >
104114
// qqq : don't use 1-prameter Result
105115
{
106116
let path = AbsolutePath::try_from( o.path )?;
107117
let dir = CrateDir::try_from( path.clone() )?;
108118

119+
let workspace = Workspace::try_from( dir.clone() )?;
120+
109121
let list = action::list_all
110122
(
111123
action::list::ListOptions::former()
@@ -150,7 +162,7 @@ mod private
150162
.checking_consistency( false )
151163
.dry( false ).form()
152164
)?;
153-
let l = CrateArchive::read( packed_crate::local_path( name, version, dir )? )?;
165+
let l = CrateArchive::read( packed_crate::local_path( name, version, workspace.target_directory() )? )?;
154166
let r = CrateArchive::download_crates_io( name, version ).unwrap();
155167

156168

module/move/willbe/src/command/publish.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ mod private
5555
dry,
5656
temp
5757
} = o.props.try_into()?;
58-
let plan = action::publish_plan( patterns, channel, dry, temp )
58+
let plan = action::publish_plan( &patterns, channel, dry, temp )
5959
.context( "Failed to plan the publication process" )?;
6060

6161
let mut formatted_plan = String::new();

module/move/willbe/src/entity/package.rs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -208,24 +208,44 @@ mod private
208208

209209
//
210210

211-
/// Determines whether a package needs to be published by comparing `.crate` files from the local and remote package.
211+
/// Determines if a package needs to be published by comparing its local `.crate` file against the version on crates.io.
212212
///
213-
/// This function requires the local package to be previously packed.
213+
/// This function first locates the local, pre-packaged `.crate` file and then attempts to download
214+
/// the corresponding version from the remote registry. It returns `true` if there are differences
215+
/// or if the remote version does not exist (implying a new version to be published).
214216
///
215-
/// # Returns :
216-
/// - `true` if the package needs to be published.
217-
/// - `false` if there is no need to publish the package.
217+
/// **Prerequisite**: The local package must have been packaged beforehand (e.g., using `cargo package`).
218+
///
219+
/// # Arguments
220+
///
221+
/// * `package` - A reference to the `Package` struct for which the check is being performed.
222+
/// * `path` - An optional path to a directory that contains the packaged `.crate` file.
223+
/// If `Some`, this path is used directly. If `None`, the path is constructed using `target_dir`.
224+
/// * `target_dir` - The path to the workspace's `target` directory, used to find the
225+
/// local `.crate` file if a specific `path` is not provided.
226+
///
227+
/// # Returns
228+
///
229+
/// - `Ok(true)` if the local and remote `.crate` files have differences, or if the package
230+
/// version does not exist on crates.io (e.g., a 403 Forbidden error is received).
231+
/// - `Ok(false)` if the local and remote packages are identical.
218232
///
219-
/// Panics if the package is not loaded or local package is not packed.
220233
/// # Errors
221-
/// qqq: doc
222-
pub fn publish_need( package : &Package< '_ >, path : Option< path::PathBuf > ) -> Result< bool, PackageError >
234+
///
235+
/// This function will return an error in the following cases:
236+
///
237+
/// - `PackageError::LocalPath`: If the path to the local `.crate` file cannot be determined.
238+
/// - `PackageError::ReadArchive`: If the local `.crate` file exists but cannot be read.
239+
/// - `PackageError::LoadRemotePackage`: If downloading the remote package fails for reasons
240+
/// other than a non-existent version (e.g., network issues).
241+
/// - Any error that occurs while trying to read the package's name or version.
242+
pub fn publish_need( package : &Package< '_ >, path : Option< path::PathBuf >, target_dir : &std::path::Path ) -> Result< bool, PackageError >
223243
{
224244
let name = package.name()?;
225245
let version = package.version()?;
226246
let local_package_path = path
227247
.map( | p | p.join( format!( "package/{name}-{version}.crate" ) ) )
228-
.unwrap_or( packed_crate::local_path( name, &version, package.crate_dir() ).map_err( | _ | PackageError::LocalPath )? );
248+
.unwrap_or( packed_crate::local_path( name, &version, target_dir ).map_err( | _ | PackageError::LocalPath )? );
229249

230250
let local_package = CrateArchive::read( local_package_path ).map_err( | _ | PackageError::ReadArchive )?;
231251
let remote_package = match CrateArchive::download_crates_io( name, version )

module/move/willbe/src/entity/packed_crate.rs

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,29 +13,34 @@ mod private
1313
use error::{ untyped::Context };
1414
use ureq::Agent;
1515

16-
/// Returns the local path of a packed `.crate` file based on its name, version, and manifest path.
16+
/// Constructs the expected local path for a packed `.crate` file within a target directory.
1717
///
18-
/// # Args :
19-
/// - `name` - the name of the package.
20-
/// - `version` - the version of the package.
21-
/// - `manifest_file` - path to the package `Cargo.toml` file.
18+
/// This is a utility function that builds a predictable path without verifying
19+
/// if the file actually exists. It follows the standard Cargo packaging structure.
2220
///
23-
/// # Returns :
24-
/// The local packed `.crate` file of the package
21+
/// # Arguments
22+
///
23+
/// - `name` - The name of the package.
24+
/// - `version` - The version of the package.
25+
/// - `target_dir` - The path to the workspace's `target` directory, inside which
26+
/// the `package/` subdirectory is expected.
27+
///
28+
/// # Returns
29+
///
30+
/// Returns a `Result` containing a `PathBuf` that points to the expected location of the `.crate` file,
31+
/// for example: `<target_dir>/package/my_package-0.1.0.crate`.
2532
///
2633
/// # Errors
27-
/// qqq: doc
34+
///
35+
/// This function is currently infallible as it only performs path joining and string formatting.
36+
/// The `Result` is kept for API consistency.
2837
// qqq : typed error
29-
pub fn local_path< 'a >( name : &'a str, version : &'a str, crate_dir : CrateDir ) -> error::untyped::Result< PathBuf >
38+
pub fn local_path< 'a >( name : &'a str, version : &'a str, target_dir : &std::path::Path ) -> error::untyped::Result< PathBuf >
3039
{
3140
let buf = format!( "package/{name}-{version}.crate" );
32-
let workspace = Workspace::try_from( crate_dir )?;
33-
34-
let mut local_package_path = PathBuf::new();
35-
local_package_path.push( workspace.target_directory() );
36-
local_package_path.push( buf );
37-
41+
let local_package_path = target_dir.join( buf );
3842
Ok( local_package_path )
43+
3944
}
4045

4146
///

module/move/willbe/src/entity/publish.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ mod private
390390
};
391391
report.add = git.add;
392392
report.commit = git.commit;
393-
report.publish = match cargo::publish( publish )
393+
report.publish = match cargo::publish( &publish )
394394
{
395395
Ok( publish ) => Some( publish ),
396396
Err( e ) =>

module/move/willbe/src/tool/cargo.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,14 @@ mod private
7575
#[ allow( clippy::if_not_else ) ]
7676
fn to_pack_args( &self ) -> Vec< String >
7777
{
78+
// Building the full path to Cargo.toml
79+
let manifest_path = self.path.join( "Cargo.toml" );
80+
let normalized_manifest_path = manifest_path.to_string_lossy().replace( '\\', "/" );
7881
[ "run".to_string(), self.channel.to_string(), "cargo".into(), "package".into() ]
7982
.into_iter()
83+
// clearly show the way to the manifesto
84+
.chain( Some( "--manifest-path".to_string() ) )
85+
.chain( Some( normalized_manifest_path ) )
8086
.chain( if self.allow_dirty { Some( "--allow-dirty".to_string() ) } else { None } )
8187
.chain( if !self.checking_consistency { Some( "--no-verify".to_string() ) } else { None } )
8288
.chain( self.temp_path.clone().map( | p | vec![ "--target-dir".to_string(), p.to_string_lossy().into() ] ).into_iter().flatten() )
@@ -92,6 +98,11 @@ mod private
9298
/// - `path` - path to the package directory
9399
/// - `dry` - a flag that indicates whether to execute the command or not
94100
///
101+
// FIX: Added # Errors section for `pack` function
102+
/// # Errors
103+
///
104+
/// Returns an error if the `rustup ... cargo package` command fails.
105+
///
95106
#[ cfg_attr
96107
(
97108
feature = "tracing",
@@ -159,14 +170,19 @@ mod private
159170
}
160171
}
161172

162-
/// Upload a package to the registry
173+
/// Upload a package to the registry
174+
// FIX: Added # Errors section for `publish` function
175+
/// # Errors
176+
///
177+
/// Returns an error if the `cargo publish` command fails after all retry attempts.
178+
///
163179
#[ cfg_attr
164180
(
165181
feature = "tracing",
166182
track_caller,
167183
tracing::instrument( fields( caller = ?{ let x = std::panic::Location::caller(); ( x.file(), x.line() ) } ) )
168184
)]
169-
pub fn publish( args : PublishOptions ) -> error::untyped::Result< process::Report >
185+
pub fn publish( args : &PublishOptions ) -> error::untyped::Result< process::Report >
170186
// qqq : use typed error
171187
{
172188

module/move/willbe/src/tool/git.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ mod private
2424
///
2525
/// # Returns :
2626
/// Returns a result containing a report indicating the result of the operation.
27+
/// # Errors
28+
///
29+
/// Returns an error if the `git add` command fails.
2730
// qqq : should be typed error, apply err_with
2831
#[ cfg_attr
2932
(
@@ -79,6 +82,9 @@ mod private
7982
///
8083
/// # Returns :
8184
/// Returns a result containing a report indicating the result of the operation.
85+
/// # Errors
86+
///
87+
/// Returns an error if the `git commit` command fails.
8288
// qqq : should be typed error, apply err_with
8389
#[ cfg_attr
8490
(
@@ -132,7 +138,9 @@ mod private
132138
///
133139
/// # Returns :
134140
/// Returns a result containing a report indicating the result of the operation.
135-
141+
/// # Errors
142+
///
143+
/// Returns an error if the `git push` command fails.
136144
// qqq : should be typed error, apply err_with
137145

138146
#[ cfg_attr( feature = "tracing", tracing::instrument( skip( path ), fields( path = %path.as_ref().display() ) ) ) ]

0 commit comments

Comments
 (0)