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.

Sign up for GitHub

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

Jump to bottom

Fixes #26340: Write a new test framework for the Rudder methods #6174

Open
wants to merge 2 commits into
base: branches/rudder/8.3
Choose a base branch
from
Open

Fixes #26340: Write a new test framework for the Rudder methods #6174

wants to merge 2 commits into from

Conversation

Fdall
Copy link
Contributor

@Fdall Fdall commented Feb 11, 2025

@Fdall Fdall requested a review from amousset February 11, 2025 09:38
@Fdall
Copy link
Contributor Author

Fdall commented Feb 11, 2025

Commit modified

@Fdall Fdall force-pushed the ust_26340/write_a_new_test_framework_for_the_rudder_methods branch from ccd0318 to 001d061 Compare February 11, 2025 09:48
@Fdall
Copy link
Contributor Author

Fdall commented Feb 11, 2025

PR updated with a new commit

amousset
amousset reviewed Feb 11, 2025
View reviewed changes
policies/lib/Cargo.toml Outdated
@@ -0,0 +1,18 @@
[package]
name = "rudder-methods"
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to have the same name as the folder. We could rename it to "methods", or rename the library to "lib".

amousset
amousset reviewed Feb 11, 2025
View reviewed changes
policies/lib/src/file_check_exists.rs Outdated
@@ -0,0 +1,43 @@
extern crate rudderc;
Copy link
Member

Choose a reason for hiding this comment

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

no need for this in edition 2021

amousset
amousset reviewed Feb 11, 2025
View reviewed changes
policies/lib/src/method_test_suite.rs Outdated
}
}

#[allow(dead_code)]
Copy link
Member

Choose a reason for hiding this comment

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

you can put a #![allow(dead_code)] at the crate root

@Fdall
Copy link
Contributor Author

Fdall commented Feb 11, 2025

PR updated with a new commit

1 similar comment
@Fdall
Copy link
Contributor Author

Fdall commented Feb 11, 2025

PR updated with a new commit

amousset
amousset reviewed Feb 11, 2025
View reviewed changes
policies/lib/Cargo.toml Outdated

[dependencies]
serde = { version = "1", features = ["derive"] }
rudderc = { path = "../rudderc", features = ["embedded-lib"] }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
rudderc = { path = "../rudderc", features = ["embedded-lib"] }
rudderc = { path = "../rudderc" }

I don't think we want this feature here.

@Fdall
Copy link
Contributor Author

Fdall commented Feb 11, 2025

PR updated with a new commit

7 similar comments
@Fdall
Copy link
Contributor Author

Fdall commented Feb 11, 2025

PR updated with a new commit

@Fdall
Copy link
Contributor Author

Fdall commented Feb 11, 2025

PR updated with a new commit

@Fdall
Copy link
Contributor Author

Fdall commented Feb 12, 2025

PR updated with a new commit

@Fdall
Copy link
Contributor Author

Fdall commented Feb 12, 2025

PR updated with a new commit

@Fdall
Copy link
Contributor Author

Fdall commented Feb 12, 2025

PR updated with a new commit

@Fdall
Copy link
Contributor Author

Fdall commented Feb 12, 2025

PR updated with a new commit

@Fdall
Copy link
Contributor Author

Fdall commented Feb 12, 2025

PR updated with a new commit

@Fdall
Copy link
Contributor Author

Fdall commented Feb 12, 2025

PR rebased

@Fdall Fdall force-pushed the ust_26340/write_a_new_test_framework_for_the_rudder_methods branch from 143d52b to 0249ddf Compare February 12, 2025 11:41
@Fdall
Copy link
Contributor Author

Fdall commented Feb 12, 2025

PR updated with a new commit

@Fdall
Copy link
Contributor Author

Fdall commented Feb 12, 2025

PR rebased

@Fdall Fdall force-pushed the ust_26340/write_a_new_test_framework_for_the_rudder_methods branch from 6d529e1 to a801eed Compare February 12, 2025 18:07
amousset
amousset reviewed Feb 13, 2025
View reviewed changes
policies/rudderc/src/backends/unix/cfengine.rs Outdated
@@ -146,13 +147,38 @@ pub fn cfengine_canonify_condition(c: &str) -> String {
}
}

#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, Hash)]
pub struct CfEngineDatastate {
#[serde(rename = "classes")]
Copy link
Member

Choose a reason for hiding this comment

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

no need for rename if the name is the same

@Fdall
Copy link
Contributor Author

Fdall commented Feb 14, 2025

PR updated with a new commit

1 similar comment
@Fdall
Copy link
Contributor Author

Fdall commented Feb 14, 2025

PR updated with a new commit

amousset
amousset reviewed Feb 17, 2025
View reviewed changes
@Fdall
Copy link
Contributor Author

Fdall commented Feb 17, 2025

PR updated with a new commit

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/6174
-- Your faithful QA
Kant merge: "Science is organized knowledge. Wisdom is organized life."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/97099/console)

@amousset
Copy link
Member

OK, squash merging this PR

@amousset amousset force-pushed the ust_26340/write_a_new_test_framework_for_the_rudder_methods branch from a91fd43 to 4ee491a Compare February 18, 2025 13:10
@amousset
Copy link
Member

Needs a rebase.

@Fdall
Copy link
Contributor Author

Fdall commented Feb 19, 2025

PR rebased

@Fdall Fdall force-pushed the ust_26340/write_a_new_test_framework_for_the_rudder_methods branch from 4ee491a to 3f27451 Compare February 19, 2025 09:45
@Fdall
Copy link
Contributor Author

Fdall commented Feb 19, 2025

PR rebased

@Fdall Fdall force-pushed the ust_26340/write_a_new_test_framework_for_the_rudder_methods branch from 3f27451 to 6879ff1 Compare February 19, 2025 11:35
fixup! Fixes #26340: Write a new test framework for the Rudder methods
b108879 
Fixes #26340: Write a new test framework for the Rudder methods
@Fdall
Copy link
Contributor Author

Fdall commented Feb 19, 2025

PR updated with a new commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amousset amousset amousset approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Fdall @Normation-Quality-Assistant @amousset