Page MenuHomeFreeBSD
Differential D15751

Add a single test-program for praudit(1) utility
ClosedPublic
Actions

Authored by aniketp on Jun 10 2018, 11:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Feb 13, 8:59 AM
Unknown Object (File)
Thu, Feb 12, 7:53 PM
Unknown Object (File)
Thu, Feb 12, 12:11 PM
Unknown Object (File)
Wed, Feb 11, 9:52 PM
Unknown Object (File)
Wed, Feb 11, 9:01 PM
Unknown Object (File)
Wed, Feb 11, 12:01 PM
Unknown Object (File)
Wed, Feb 11, 10:13 AM
Unknown Object (File)
Wed, Feb 11, 7:03 AM
View All Files
Subscribers
imp

Details

Reviewers
asomers
rwatson
Commits
rS337256: MFC r335287, r335290
rS336212: MFC r335287, r335290
rS335290: praudit(1): add tests
Summary

The revision introduces a new test-program for an audit viewer application: praudit.
The tests have been created for checking the proper functioning of each mode of execution.

-d del  Specifies the delimiter.  The default delimiter is the comma.

-l      Prints the entire record on the same line.  If this option is not
        specified, every token is displayed on a different line.

-n      Do not convert user and group IDs to their names but leave in
        their numeric forms.

-p      Specify this option if input to praudit is piped from the tail(1)
        utility.  This causes praudit to sync to the start of the next
        record.

-r      Prints the records in their raw, numeric form.  This option is
        exclusive from -s.

-s      Prints the tokens in their short form.  Short text
        representations for record and event type are displayed.  This
        option is exclusive from -r.

-x      Print audit records in the XML output format.
NOTE: The above snippet is taken from praudit.1
Test Plan

Execute make install from usr.sbin/praudit/tests.
Execute kyua test from /usr/test/usr.sbin/praudit. The test case should succeed.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17225
Build 17070: arc lint + arc unit

Event Timeline

aniketp created this revision.Jun 10 2018, 11:36 PM
Harbormaster completed remote builds in B17156: Diff 43574.Jun 10 2018, 11:36 PM
asomers requested changes to this revision.Jun 11 2018, 12:16 AM

What about -p? And how about asserting the exclusivity of -s and -r in a negative test?

usr.sbin/praudit/tests/Makefile
19

Remove the final backslash

usr.sbin/praudit/tests/praudit_test.sh
34

As these assertions are all independent, you should split them into separate test cases.

38

Don't assert that the exit status is 1. That's a bug. Fix the bug instead.

This revision now requires changes to proceed.Jun 11 2018, 12:16 AM
aniketp updated this revision to Diff 43580.Jun 11 2018, 2:25 AM
  • Add a test for praudit exclusivity for 'r' and 's' arguments
  • Separate the test cases from one another
Harbormaster completed remote builds in B17160: Diff 43580.Jun 11 2018, 2:25 AM
aniketp updated this revision to Diff 43581.Jun 11 2018, 2:29 AM

Update the praudit_raw_short_exclusive's description

Harbormaster completed remote builds in B17161: Diff 43581.Jun 11 2018, 2:29 AM
aniketp updated this revision to Diff 43592.Jun 11 2018, 10:32 AM
  • Replace the audit startup trail with socket(2) trail
  • Improvements in the raw_short_exclusive description
Harbormaster completed remote builds in B17168: Diff 43592.Jun 11 2018, 10:32 AM
asomers added a comment.Jun 12 2018, 9:07 PM

I still think you should try to test "-p". That's the only thing standing between you and full coverage of the praudit command line.

aniketp updated this revision to Diff 43691.Jun 13 2018, 6:07 AM

Add a test case for -p option which is used with tail(1)

Harbormaster completed remote builds in B17222: Diff 43691.Jun 13 2018, 6:07 AM
aniketp updated this revision to Diff 43692.Jun 13 2018, 6:08 AM

Add a test case for -p option which is used with tail(1)

  • I had missed the corrupted record earlier
Harbormaster completed remote builds in B17223: Diff 43692.Jun 13 2018, 6:09 AM
aniketp updated this revision to Diff 43694.Jun 13 2018, 6:48 AM

Update praudit's flag '-p': Earlier the tests were failing

Harbormaster completed remote builds in B17224: Diff 43694.Jun 13 2018, 6:48 AM
aniketp updated this revision to Diff 43695.Jun 13 2018, 6:56 AM

Update praudit_sync_to_next_record test case

Harbormaster completed remote builds in B17225: Diff 43695.Jun 13 2018, 6:56 AM
asomers added inline comments.Jun 13 2018, 3:25 PM
usr.sbin/praudit/tests/praudit_test.sh
148

Is it a requirement that praudit _can't_ work with a truncated record without -p? I don't think it is. So you should probably remove the first assertion. Also, could you somewhere include a textual description of the difference between the normal and the truncated records?

aniketp marked 3 inline comments as done.Jun 13 2018, 3:40 PM
aniketp added inline comments.
usr.sbin/praudit/tests/praudit_test.sh
148

@asomers I had added extra random texts in the trail (before correct audit record) and verified that
praudit trail resulted in empty STDOUT
while praudit -p trail gave only the correct (later) part of the stdout.

aniketp updated this revision to Diff 43741.Jun 14 2018, 3:19 AM

Remove the normal praudit atf_check assertion in the praudit_sync_to_next_record test-case

Harbormaster completed remote builds in B17252: Diff 43741.Jun 14 2018, 3:19 AM
asomers added a comment.Jun 14 2018, 1:40 PM

I would still like to see a comment describing the contents of the corrupted file.

aniketp updated this revision to Diff 43769.Jun 14 2018, 4:29 PM

Add a description on the intent of testing the -p option

Harbormaster completed remote builds in B17274: Diff 43769.Jun 14 2018, 4:29 PM
asomers added inline comments.Jun 15 2018, 2:27 PM
usr.sbin/praudit/tests/praudit_test.sh
150

I doesn't read from the end. It still reads from the front; but it syncs to the start of the first whole record before it does anything else.

aniketp updated this revision to Diff 43829.Jun 15 2018, 4:42 PM

Update the test case description

Harbormaster completed remote builds in B17306: Diff 43829.Jun 15 2018, 4:42 PM
aniketp retitled this revision from Add a single test-case for praudit(1) utility to Add a single test-program for praudit(1) utility.Jun 15 2018, 4:44 PM
aniketp set the repository for this revision to rS FreeBSD src repository - subversion.
Herald added a subscriber: imp. · View Herald TranscriptJun 15 2018, 4:44 PM
asomers added a comment.Jun 15 2018, 5:00 PM

The praudit_test:praudit_sync_to_next_record spins the CPU and never returns. Does it do that for you too?

aniketp added a comment.Jun 16 2018, 2:05 AM

No @asomers ,
In its current form it passes smoothly:

praudit_test:praudit_sync_to_next_record  ->  passed  [0.027s]
asomers requested changes to this revision.Jun 16 2018, 2:18 PM

Since praudit -p occasionally spins the cpu (as confirmed by Aniket on IRC) you should set a shorter timeout. Apart from that this review looks good. However, it does depend on the change to praudit's return code, which hasn't been committed yet.

This revision now requires changes to proceed.Jun 16 2018, 2:18 PM
aniketp added a comment.Jun 16 2018, 3:02 PM

No @asomers, praudit -p never spins the CPU in normal invocation. (Apologies if my earlier statement was unclear)
This is the observation:

Case 1) For the tail(1) usage:

	atf_check -o file:$(atf_get_srcdir)/no_args \
		tail $(atf_get_srcdir)/corrupted | praudit -p

CPU usage shoots up and test case never returns.

Case 2) For the normal usage:

	atf_check -o file:$(atf_get_srcdir)/no_args \
		praudit -p $(atf_get_srcdir)/corrupted

Result is successful everytime.

aniketp updated this revision to Diff 43902.Jun 16 2018, 3:47 PM
  • Reduce the timeout to 2 seconds
Harbormaster completed remote builds in B17345: Diff 43902.Jun 16 2018, 3:47 PM
asomers accepted this revision.Jun 17 2018, 5:01 PM

I figured out why it works for you but not for me; it's arcanist's fault. Arcanist apparently doesn't like binary files. When I do "arc patch" it creates corrupted and trail as 0-length files. That means that a repro case for the bug is "touch /tmp/empty; praudit -p /tmp/empty".

I'm accepting this revision, but it still depends on fixing praudit's exit status.

This revision is now accepted and ready to land.Jun 17 2018, 5:01 PM
Closed by commit rS335290: praudit(1): add tests (authored by asomers). · Explain WhyJun 17 2018, 5:31 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
etc/
mtree/
2 lines
usr.sbin/
praudit/
3 lines
tests/
21 lines
input/
7 lines
7 lines
7 lines
7 lines
7 lines
1 line
7 lines
12 lines
180 lines

Diff 43695

View Options

etc/mtree/BSD.tests.dist

Loading...
View Options

usr.sbin/praudit/Makefile

Loading...
View Options

usr.sbin/praudit/tests/Makefile

Loading...
View Options

usr.sbin/praudit/tests/input/corrupted

Loading...
View Options

usr.sbin/praudit/tests/input/del_comma

Loading...
View Options

usr.sbin/praudit/tests/input/del_underscore

Loading...
View Options

usr.sbin/praudit/tests/input/no_args

Loading...
View Options

usr.sbin/praudit/tests/input/numeric_form

Loading...
View Options

usr.sbin/praudit/tests/input/raw_form

Loading...
View Options

usr.sbin/praudit/tests/input/same_line

Loading...
View Options

usr.sbin/praudit/tests/input/short_form

Loading...
View Options

usr.sbin/praudit/tests/input/trail

Loading...
View Options

usr.sbin/praudit/tests/input/xml_form

Loading...
View Options

usr.sbin/praudit/tests/praudit_test.sh

Loading...