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

CFE-3020: ps output line ... is shorter than its associated header #5082

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions tests/unit/split_process_line_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,42 @@ static void test_split_line_noelapsed(void)
free_and_null_strings(field);
}

static void test_split_line_nocmd(void)
{
/* Collect all test data in one array to make alignments visible: */
static const char *lines[] = {
/* Indent any continuation lines so that it's clear that's what they are. */
/* Use the username field as a test name to confirm tests are in sync. */

"USER PID STAT VSZ NI RSS NLWP STIME ELAPSED TIME COMMAND",
Copy link
Member

Choose a reason for hiding this comment

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

If this is exactly copied from a (ps) command, putting the command in a comment here would be helpful.

If not, is it intentional / important that the labels and values are sometimes misaligned? It could help readability if you added some spaces to align things perfectly.

"spacecmd 4 S 0 0 0 1 10:30 54:29 00:00:01 ",
"nullcmd 4 S 0 0 0 1 10:30 54:29 00:00:01 "
Comment on lines +128 to +129
Copy link
Member

Choose a reason for hiding this comment

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

Why does line 1 end with 2 spaces and line 2 end with 1 space?

};
char *name[CF_PROCCOLS] = { 0 }; /* Headers */
Copy link
Member

Choose a reason for hiding this comment

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

headers or labels would be a better name here.

char *field[CF_PROCCOLS] = { 0 }; /* Content */
int start[CF_PROCCOLS] = { 0 };
int end[CF_PROCCOLS] = { 0 };
int user = 0, command = 10;
time_t pstime = 1410000000;

/* Prepare data needed by tests and assert things assumed by test: */
GetProcessColumnNames(lines[0], name, start, end);
assert_string_equal(name[user], "USER");
assert_string_equal(name[command], "COMMAND");

assert_true(SplitProcLine(lines[1], pstime, name, start, end, PCA_AllColumnsPresent, field));
assert_string_equal(field[user], "spacecmd");
assert_string_equal(field[command], lines[1] + 69);
Copy link
Member

Choose a reason for hiding this comment

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

I think it's easier and less error prone if you just compare to what string you expect here, for example:

Suggested change
assert_string_equal(field[command], lines[1] + 69);
assert_string_equal(field[command], "");


assert_true(SplitProcLine(lines[2], pstime, name, start, end, PCA_AllColumnsPresent, field));
assert_string_equal(field[user], "nullcmd");
assert_string_equal(field[command], lines[1] + 69);
Comment on lines +147 to +149
Copy link
Member

Choose a reason for hiding this comment

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

Same as above, also unclear whether the lines[1] here is intentional or not? We are working on lines[2] now.

Suggested change
assert_true(SplitProcLine(lines[2], pstime, name, start, end, PCA_AllColumnsPresent, field));
assert_string_equal(field[user], "nullcmd");
assert_string_equal(field[command], lines[1] + 69);
assert_true(SplitProcLine(lines[2], pstime, name, start, end, PCA_AllColumnsPresent, field));
assert_string_equal(field[user], "nullcmd");
assert_string_equal(field[command], "");


/* Finally, tidy away headers: */
free_and_null_strings(name);
free_and_null_strings(field);
}

static void test_split_line_longcmd(void)
{
static const char *lines[] = {
Expand Down Expand Up @@ -909,6 +945,7 @@ int main(void)
unit_test(test_split_line_challenges),
unit_test(test_split_line_noelapsed),
unit_test(test_split_line_elapsed),
unit_test(test_split_line_nocmd),
unit_test(test_split_line_longcmd),
unit_test(test_split_line),
unit_test(test_split_line_serious_overspill),
Expand Down