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

fe_test: add test for syslog feature #5985

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

freddy77
Copy link
Collaborator

@freddy77 freddy77 commented Sep 12, 2024

The syslog feature allows to run a program and redirect its output/error stream to system log. It's triggered passing syslog_stdout argument to Forkhelpers functions like execute_command_get_output.
To test this feature use a small preload library to redirect syslog writing.
This allows to test program without changing it.
The log is redirected to /tmp/xyz instead of /dev/log. The name was chosen to allow for future static build redirection.
The C program is used only for the test, so the code style take into account this (specifically it does not try to handle all redirect situation and all error paths).

@freddy77 freddy77 requested review from lindig, edwintorok and psafont and removed request for lindig September 12, 2024 14:35
@lindig
Copy link
Contributor

lindig commented Sep 12, 2024

Where would we use this and what is the current pain point?

@edwintorok
Copy link
Contributor

there is a commented out stub for openlog in syslog_stubs.c, could we use that to redirect output for testing purposes?
LOG_PERROR would redirect messages to stderr which can be intercepted with a pipe. Not sure whether that'd be more or less convenient than intercepting with LD_PRELOAD.

@freddy77
Copy link
Collaborator Author

Okay, fixing the race, it's a problem with the test not ignoring logs from the daemon. Why it didn't happen before, I have no idea.

@freddy77
Copy link
Collaborator Author

there is a commented out stub for openlog in syslog_stubs.c, could we use that to redirect output for testing purposes? LOG_PERROR would redirect messages to stderr which can be intercepted with a pipe. Not sure whether that'd be more or less convenient than intercepting with LD_PRELOAD.

I don't want to change a line of the code beside the test, that's why I used that method.

@freddy77
Copy link
Collaborator Author

freddy77 commented Sep 12, 2024

Where would we use this and what is the current pain point?

What's "this" in your sentence ? Not clear what do you mean by pain point, are you asking me what's the importance of testing ? I suppose not. Not a rhetorical question, just I don't understand the question. That part of code was not tested so I'm adding a test.

The syslog feature allows to run a program and redirect its output/error
stream to system log. It's triggered passing "syslog_stdout" argument to
"Forkhelpers" functions like "execute_command_get_output".
To test this feature use a small preload library to redirect syslog writing.
This allows to test program without changing it.
The log is redirected to /tmp/xyz instead of /dev/log. The name was chosen
to allow for future static build redirection.
The C program is used only for the test, so the code style take into account
this (specifically it does not try to handle all redirect situation and all
error paths).

Signed-off-by: Frediano Ziglio <[email protected]>
@freddy77 freddy77 changed the title fe_test: add check for syslog feature fe_test: add test for syslog feature Sep 13, 2024
@freddy77
Copy link
Collaborator Author

Removed the race condition.
Rewrote the commit message, it was confusing, to say the least. My first though was "who wrote this?".

@freddy77
Copy link
Collaborator Author

freddy77 commented Oct 4, 2024

ping

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.

3 participants