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

ENT-2044: Added acceptance test for IP-address classes #5375

Merged
merged 1 commit into from
Nov 20, 2023

Conversation

larsewi
Copy link
Contributor

@larsewi larsewi commented Nov 14, 2023

Added acceptance test for ensuring the existence simplistic IP-address
classes

Ticket: ENT-2044
Changelog: None
Signed-off-by: Lars Erik Wik <[email protected]>
@larsewi larsewi marked this pull request as ready for review November 14, 2023 15:09

body common control
{
bundlesequence => { "test", "check" };
Copy link
Member

Choose a reason for hiding this comment

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

Looks like missing include of default.cf.sub which adds machinery for leveraging test.description and methods for pass/fail tests based on classes.

Copy link
Member

@nickanderson nickanderson left a comment

Choose a reason for hiding this comment

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

We don't strictly require it, but I think it's nice to use the dcs library. It does a few things:

  • Provides a default bundlesequence to run init, test, check if they are defined.
  • Uses test.description variable in test output
  • Provides methods for passing/failing tests based on common checks e.g. dcs_pass(test), dcs_fail(test), dcs_passif(class, test), etc ...

@larsewi
Copy link
Contributor Author

larsewi commented Nov 14, 2023

However if the test is really simple you might want to skip including
"default.cf.sub" in inputs. Then you should define your own
bundlesequence, e.g. {"test","check"}. It's recommended to avoid
including "default.cf.sub" when not needed, since the test will run much
faster.

@nickanderson, from the README I got the impression that it should be avoided for small tests? 🤔

@larsewi
Copy link
Contributor Author

larsewi commented Nov 14, 2023

But I guess the machinery for leveraging test.description makes it worth it anyways?

@nickanderson
Copy link
Member

But I guess the machinery for leveraging test.description makes it worth it anyways?

I think it does. I believe it was Jimis who added that note. And it's mostly in relation to speeding up execution IIRC. Which is valid, and why I think it's OK to have it or not. But I pretty much always use dcs. up to you.

@olehermanse
Copy link
Member

In addition to the speed argument, I like to not use the dcs stuff when the test is simple. Easier to run (just a policy file, no deps), easier to read / understand.

@larsewi
Copy link
Contributor Author

larsewi commented Nov 15, 2023

I'll keep it as is. Because it was really simple to test on Windows. I just copied the file over and ran it directly.

@larsewi larsewi merged commit 708e760 into cfengine:master Nov 20, 2023
12 checks passed
@larsewi larsewi deleted the simplistic branch December 3, 2024 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants