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

logging: Always run crate tests #2970

Merged

Conversation

jodh-intel
Copy link
Contributor

Ensure the tests in the local logging crate are run for all consumers
of it.

Additionally, add a new test which checks that output is generated by a
range of different log level slog macros. This is designed to ensure
debug level output is always available for the consumers of the
logging crate.

Fixes: #2969.

Signed-off-by: James O. D. Hunt james.o.hunt@intel.com

Fixed the top-level build which was broken: the kata deploy
Makefile was being sourced, but it was defining the first target, which
became the default.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
@jodh-intel jodh-intel force-pushed the logging-create-tests-and-checks branch from 493dcaa to 865851a Compare November 4, 2021 16:33
@jodh-intel
Copy link
Contributor Author

/test

@jodh-intel
Copy link
Contributor Author

/test

Ensure the tests in the local `logging` crate are run for all consumers
of it.

Additionally, add a new test which checks that output is generated by a
range of different log level `slog` macros. This is designed to ensure
debug level output is always available for the consumers of the
`logging` crate.

Fixes: kata-containers#2969.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
@jodh-intel jodh-intel force-pushed the logging-create-tests-and-checks branch from 829cbb8 to d47484e Compare November 4, 2021 17:27
@jodh-intel
Copy link
Contributor Author

/test

@jodh-intel
Copy link
Contributor Author

@liubin - could you tal?

@@ -17,11 +17,16 @@ TOOLS += agent-ctl

STANDARD_TARGETS = build check clean install test vendor

default: all

all: logging-crate-tests build
Copy link
Member

Choose a reason for hiding this comment

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

Does logging-crate-tests will run twice? build target in sub-directories will also trigger logging-crate-tests too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@liubin - it might yes. The problem is we need to ensure the logging crate tests run however the build is invoked - either running make from the top-level, or running make -C src/agent for example. I'm not sure how to avoid running it multiple times whilst also ensuring that it does run. Any ideas?

ftr, running the logging crate tests twice takes < 300ms on a laptop system so it's potentially unecessary to run multiple times, but it isn't going to increase the build time much.

Ideally, we want cargo test and cargo test --release to run when any of the other components run cargo build, but I don't think that's possible. I did think of one way, but it's horrid: create pkg/logging/build.rs which runs make! ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a hack, but I could then remove all the changes to the other makefiles.

@cmaf
Copy link
Contributor

cmaf commented Nov 5, 2021

/retest-vfio

@jodh-intel
Copy link
Contributor Author

CI green, so just need 1 more ack here.

@fidencio
Copy link
Member

fidencio commented Nov 8, 2021

Going through this one now, James.

Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

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

@jodh-intel, lgtm, but take this with a grain of salt considering my current rust skills!
Thanks!

@jodh-intel jodh-intel merged commit b192d38 into kata-containers:main Nov 8, 2021
@jodh-intel
Copy link
Contributor Author

Thanks @fidencio ! ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Always run logging crate tests
4 participants