* [ltt-dev] [UST PATCH] Fix libustctl_function_tests @ 2011-02-23 17:46 Yannick Brosseau 2011-02-23 18:51 ` Nils Carlson 2011-02-24 8:56 ` Nils Carlson 0 siblings, 2 replies; 8+ messages in thread From: Yannick Brosseau @ 2011-02-23 17:46 UTC (permalink / raw) After discussions, we concluded that the enable a non existing marker is a valid case, so we move it to the working case section. While being there, check that the re-enable a marker set the right errno Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com> --- .../libustctl_function_tests.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/libustctl_function_tests/libustctl_function_tests.c b/tests/libustctl_function_tests/libustctl_function_tests.c index 947028f..7c12695 100644 --- a/tests/libustctl_function_tests/libustctl_function_tests.c +++ b/tests/libustctl_function_tests/libustctl_function_tests.c @@ -21,6 +21,7 @@ #include <unistd.h> #include <sys/types.h> #include <sys/wait.h> +#include <errno.h> #include <ust/marker.h> #include <ust/ustctl.h> @@ -136,15 +137,16 @@ static void ustctl_function_tests(pid_t pid) tap_ok(!ustctl_destroy_trace(trace, pid), "ustctl_destroy_trace - without ever starting"); + tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid) == 0, + "Enable non-existent marker ustl blar"); printf("##### Tests that definetly should work are completed #####\n"); printf("############## Start expected failure cases ##############\n"); tap_ok(ustctl_set_marker_state(trace, "ust","bar", 1, pid), "Enable already enabled marker ust/bar"); - - tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid), - "Enable non-existent marker ustl blar"); + tap_ok(EEXIST == errno, + "Right error code for enabling an already enabled marker"); tap_ok(ustctl_start_trace(trace, pid), "Start a non-existent trace"); @@ -161,7 +163,7 @@ int main(int argc, char **argv) int i, status; pid_t parent_pid, child_pid; - tap_plan(27); + tap_plan(28); printf("Function tests for ustctl\n"); -- 1.7.2.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [ltt-dev] [UST PATCH] Fix libustctl_function_tests 2011-02-23 17:46 [ltt-dev] [UST PATCH] Fix libustctl_function_tests Yannick Brosseau @ 2011-02-23 18:51 ` Nils Carlson 2011-02-23 18:43 ` Mathieu Desnoyers ` (2 more replies) 2011-02-24 8:56 ` Nils Carlson 1 sibling, 3 replies; 8+ messages in thread From: Nils Carlson @ 2011-02-23 18:51 UTC (permalink / raw) On Wed, 23 Feb 2011, Yannick Brosseau wrote: > After discussions, we concluded that the enable a non existing marker is a valid case, so we > move it to the working case section. > While being there, check that the re-enable a marker set the right errno > I don't really understand your reasoning. This is a question of libust being broken and the test-case showing this. We should fix the test-case. It should not be possible to enable a non-existent marker, the function should return -1, not 0 as it currently does. /Nils > Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com> > --- > .../libustctl_function_tests.c | 10 ++++++---- > 1 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/tests/libustctl_function_tests/libustctl_function_tests.c b/tests/libustctl_function_tests/libustctl_function_tests.c > index 947028f..7c12695 100644 > --- a/tests/libustctl_function_tests/libustctl_function_tests.c > +++ b/tests/libustctl_function_tests/libustctl_function_tests.c > @@ -21,6 +21,7 @@ > #include <unistd.h> > #include <sys/types.h> > #include <sys/wait.h> > +#include <errno.h> > > #include <ust/marker.h> > #include <ust/ustctl.h> > @@ -136,15 +137,16 @@ static void ustctl_function_tests(pid_t pid) > > tap_ok(!ustctl_destroy_trace(trace, pid), "ustctl_destroy_trace - without ever starting"); > > + tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid) == 0, > + "Enable non-existent marker ustl blar"); > > printf("##### Tests that definetly should work are completed #####\n"); > printf("############## Start expected failure cases ##############\n"); > > tap_ok(ustctl_set_marker_state(trace, "ust","bar", 1, pid), > "Enable already enabled marker ust/bar"); > - > - tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid), > - "Enable non-existent marker ustl blar"); > + tap_ok(EEXIST == errno, > + "Right error code for enabling an already enabled marker"); > > tap_ok(ustctl_start_trace(trace, pid), > "Start a non-existent trace"); > @@ -161,7 +163,7 @@ int main(int argc, char **argv) > int i, status; > pid_t parent_pid, child_pid; > > - tap_plan(27); > + tap_plan(28); > > printf("Function tests for ustctl\n"); > > -- > 1.7.2.3 > > > _______________________________________________ > ltt-dev mailing list > ltt-dev at lists.casi.polymtl.ca > http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [ltt-dev] [UST PATCH] Fix libustctl_function_tests 2011-02-23 18:51 ` Nils Carlson @ 2011-02-23 18:43 ` Mathieu Desnoyers 2011-02-23 18:56 ` Nils Carlson [not found] ` <BLU0-SMTP641C88D30CC26BE35858F96DB0@phx.gbl> 2 siblings, 0 replies; 8+ messages in thread From: Mathieu Desnoyers @ 2011-02-23 18:43 UTC (permalink / raw) * Nils Carlson (nils.carlson at ericsson.com) wrote: > On Wed, 23 Feb 2011, Yannick Brosseau wrote: > >> After discussions, we concluded that the enable a non existing marker is a valid case, so we >> move it to the working case section. >> While being there, check that the re-enable a marker set the right errno >> > > I don't really understand your reasoning. This is a question of libust > being broken and the test-case showing this. We should fix the test-case. > It should not be possible to enable a non-existent marker, the function > should return -1, not 0 as it currently does. Enabling a non-existent marker/tracepoint is a perfectly valid case. If the application later on loads a library with dlopen that contains this marker, it should be enabled as soon as the library is loaded. So this requires that the user can ask for enabling markers that do not appear in the marker list yet. I do that to handle module loading in LTTng (kernel). Thanks, Mathieu > > /Nils > >> Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com> >> --- >> .../libustctl_function_tests.c | 10 ++++++---- >> 1 files changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/tests/libustctl_function_tests/libustctl_function_tests.c b/tests/libustctl_function_tests/libustctl_function_tests.c >> index 947028f..7c12695 100644 >> --- a/tests/libustctl_function_tests/libustctl_function_tests.c >> +++ b/tests/libustctl_function_tests/libustctl_function_tests.c >> @@ -21,6 +21,7 @@ >> #include <unistd.h> >> #include <sys/types.h> >> #include <sys/wait.h> >> +#include <errno.h> >> >> #include <ust/marker.h> >> #include <ust/ustctl.h> >> @@ -136,15 +137,16 @@ static void ustctl_function_tests(pid_t pid) >> >> tap_ok(!ustctl_destroy_trace(trace, pid), "ustctl_destroy_trace - without ever starting"); >> >> + tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid) == 0, >> + "Enable non-existent marker ustl blar"); >> >> printf("##### Tests that definetly should work are completed #####\n"); >> printf("############## Start expected failure cases ##############\n"); >> >> tap_ok(ustctl_set_marker_state(trace, "ust","bar", 1, pid), >> "Enable already enabled marker ust/bar"); >> - >> - tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid), >> - "Enable non-existent marker ustl blar"); >> + tap_ok(EEXIST == errno, >> + "Right error code for enabling an already enabled marker"); >> >> tap_ok(ustctl_start_trace(trace, pid), >> "Start a non-existent trace"); >> @@ -161,7 +163,7 @@ int main(int argc, char **argv) >> int i, status; >> pid_t parent_pid, child_pid; >> >> - tap_plan(27); >> + tap_plan(28); >> >> printf("Function tests for ustctl\n"); >> >> -- >> 1.7.2.3 >> >> >> _______________________________________________ >> ltt-dev mailing list >> ltt-dev at lists.casi.polymtl.ca >> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev >> > > _______________________________________________ > ltt-dev mailing list > ltt-dev at lists.casi.polymtl.ca > http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev > -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* [ltt-dev] [UST PATCH] Fix libustctl_function_tests 2011-02-23 18:51 ` Nils Carlson 2011-02-23 18:43 ` Mathieu Desnoyers @ 2011-02-23 18:56 ` Nils Carlson [not found] ` <BLU0-SMTP641C88D30CC26BE35858F96DB0@phx.gbl> 2 siblings, 0 replies; 8+ messages in thread From: Nils Carlson @ 2011-02-23 18:56 UTC (permalink / raw) On Wed, 23 Feb 2011, Nils Carlson wrote: > On Wed, 23 Feb 2011, Yannick Brosseau wrote: > >> After discussions, we concluded that the enable a non existing marker is a valid case, so we >> move it to the working case section. >> While being there, check that the re-enable a marker set the right errno >> > > I don't really understand your reasoning. This is a question of libust > being broken and the test-case showing this. We should fix the test-case. Oh, any by fix the test-case I actually meant fix libust... :-) We should be getting -1 here, not 0 when enabling a non-existent marker. /Nils > It should not be possible to enable a non-existent marker, the function > should return -1, not 0 as it currently does. > > /Nils > >> Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com> >> --- >> .../libustctl_function_tests.c | 10 ++++++---- >> 1 files changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/tests/libustctl_function_tests/libustctl_function_tests.c b/tests/libustctl_function_tests/libustctl_function_tests.c >> index 947028f..7c12695 100644 >> --- a/tests/libustctl_function_tests/libustctl_function_tests.c >> +++ b/tests/libustctl_function_tests/libustctl_function_tests.c >> @@ -21,6 +21,7 @@ >> #include <unistd.h> >> #include <sys/types.h> >> #include <sys/wait.h> >> +#include <errno.h> >> >> #include <ust/marker.h> >> #include <ust/ustctl.h> >> @@ -136,15 +137,16 @@ static void ustctl_function_tests(pid_t pid) >> >> tap_ok(!ustctl_destroy_trace(trace, pid), "ustctl_destroy_trace - without ever starting"); >> >> + tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid) == 0, >> + "Enable non-existent marker ustl blar"); >> >> printf("##### Tests that definetly should work are completed #####\n"); >> printf("############## Start expected failure cases ##############\n"); >> >> tap_ok(ustctl_set_marker_state(trace, "ust","bar", 1, pid), >> "Enable already enabled marker ust/bar"); >> - >> - tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid), >> - "Enable non-existent marker ustl blar"); >> + tap_ok(EEXIST == errno, >> + "Right error code for enabling an already enabled marker"); >> >> tap_ok(ustctl_start_trace(trace, pid), >> "Start a non-existent trace"); >> @@ -161,7 +163,7 @@ int main(int argc, char **argv) >> int i, status; >> pid_t parent_pid, child_pid; >> >> - tap_plan(27); >> + tap_plan(28); >> >> printf("Function tests for ustctl\n"); >> >> -- >> 1.7.2.3 >> >> >> _______________________________________________ >> ltt-dev mailing list >> ltt-dev at lists.casi.polymtl.ca >> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev >> > > _______________________________________________ > ltt-dev mailing list > ltt-dev at lists.casi.polymtl.ca > http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev > ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <BLU0-SMTP641C88D30CC26BE35858F96DB0@phx.gbl>]
* [ltt-dev] [UST PATCH] Fix libustctl_function_tests [not found] ` <BLU0-SMTP641C88D30CC26BE35858F96DB0@phx.gbl> @ 2011-02-23 20:54 ` Nils Carlson 2011-02-23 20:56 ` Mathieu Desnoyers [not found] ` <BLU0-SMTP5860CC88B2AC6EDE7D2AEE96DB0@phx.gbl> 0 siblings, 2 replies; 8+ messages in thread From: Nils Carlson @ 2011-02-23 20:54 UTC (permalink / raw) On Wed, 23 Feb 2011, Mathieu Desnoyers wrote: > * Nils Carlson (nils.carlson at ericsson.com) wrote: >> On Wed, 23 Feb 2011, Yannick Brosseau wrote: >> >>> After discussions, we concluded that the enable a non existing marker is a valid case, so we >>> move it to the working case section. >>> While being there, check that the re-enable a marker set the right errno >>> >> >> I don't really understand your reasoning. This is a question of libust >> being broken and the test-case showing this. We should fix the test-case. >> It should not be possible to enable a non-existent marker, the function >> should return -1, not 0 as it currently does. > > Enabling a non-existent marker/tracepoint is a perfectly valid case. If > the application later on loads a library with dlopen that contains this > marker, it should be enabled as soon as the library is loaded. So this > requires that the user can ask for enabling markers that do not appear > in the marker list yet. I do that to handle module loading in LTTng > (kernel). Ok. So we can pre-activate a marker... Interesting. Then I agree we can change the test-case. Though maybe somebody should document this as it doesn't make a lot of sense unless you consider the library case. Also, I'm not really sure I like it. If a user expects a marker to be there and the marker isn't it feels silly not to give any feedback. I could see a lot of users expecting ust to complain when you activate non-existent markers, following the rule of least-suprise I think that it should complain. Have you considered adding a parameter for this? Like a FORCE_ACTIVATE flag or something? /Nils > Thanks, > > Mathieu > >> >> /Nils >> >>> Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com> >>> --- >>> .../libustctl_function_tests.c | 10 ++++++---- >>> 1 files changed, 6 insertions(+), 4 deletions(-) >>> >>> diff --git a/tests/libustctl_function_tests/libustctl_function_tests.c b/tests/libustctl_function_tests/libustctl_function_tests.c >>> index 947028f..7c12695 100644 >>> --- a/tests/libustctl_function_tests/libustctl_function_tests.c >>> +++ b/tests/libustctl_function_tests/libustctl_function_tests.c >>> @@ -21,6 +21,7 @@ >>> #include <unistd.h> >>> #include <sys/types.h> >>> #include <sys/wait.h> >>> +#include <errno.h> >>> >>> #include <ust/marker.h> >>> #include <ust/ustctl.h> >>> @@ -136,15 +137,16 @@ static void ustctl_function_tests(pid_t pid) >>> >>> tap_ok(!ustctl_destroy_trace(trace, pid), "ustctl_destroy_trace - without ever starting"); >>> >>> + tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid) == 0, >>> + "Enable non-existent marker ustl blar"); >>> >>> printf("##### Tests that definetly should work are completed #####\n"); >>> printf("############## Start expected failure cases ##############\n"); >>> >>> tap_ok(ustctl_set_marker_state(trace, "ust","bar", 1, pid), >>> "Enable already enabled marker ust/bar"); >>> - >>> - tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid), >>> - "Enable non-existent marker ustl blar"); >>> + tap_ok(EEXIST == errno, >>> + "Right error code for enabling an already enabled marker"); >>> >>> tap_ok(ustctl_start_trace(trace, pid), >>> "Start a non-existent trace"); >>> @@ -161,7 +163,7 @@ int main(int argc, char **argv) >>> int i, status; >>> pid_t parent_pid, child_pid; >>> >>> - tap_plan(27); >>> + tap_plan(28); >>> >>> printf("Function tests for ustctl\n"); >>> >>> -- >>> 1.7.2.3 >>> >>> >>> _______________________________________________ >>> ltt-dev mailing list >>> ltt-dev at lists.casi.polymtl.ca >>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev >>> >> >> _______________________________________________ >> ltt-dev mailing list >> ltt-dev at lists.casi.polymtl.ca >> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev >> > > -- > Mathieu Desnoyers > Operating System Efficiency R&D Consultant > EfficiOS Inc. > http://www.efficios.com > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [ltt-dev] [UST PATCH] Fix libustctl_function_tests 2011-02-23 20:54 ` Nils Carlson @ 2011-02-23 20:56 ` Mathieu Desnoyers [not found] ` <BLU0-SMTP5860CC88B2AC6EDE7D2AEE96DB0@phx.gbl> 1 sibling, 0 replies; 8+ messages in thread From: Mathieu Desnoyers @ 2011-02-23 20:56 UTC (permalink / raw) * Nils Carlson (nils.carlson at ericsson.com) wrote: > On Wed, 23 Feb 2011, Mathieu Desnoyers wrote: > >> * Nils Carlson (nils.carlson at ericsson.com) wrote: >>> On Wed, 23 Feb 2011, Yannick Brosseau wrote: >>> >>>> After discussions, we concluded that the enable a non existing marker is a valid case, so we >>>> move it to the working case section. >>>> While being there, check that the re-enable a marker set the right errno >>>> >>> >>> I don't really understand your reasoning. This is a question of libust >>> being broken and the test-case showing this. We should fix the test-case. >>> It should not be possible to enable a non-existent marker, the function >>> should return -1, not 0 as it currently does. >> >> Enabling a non-existent marker/tracepoint is a perfectly valid case. If >> the application later on loads a library with dlopen that contains this >> marker, it should be enabled as soon as the library is loaded. So this >> requires that the user can ask for enabling markers that do not appear >> in the marker list yet. I do that to handle module loading in LTTng >> (kernel). > > Ok. So we can pre-activate a marker... Interesting. Then I agree we can > change the test-case. Though maybe somebody should document this as it > doesn't make a lot of sense unless you consider the library case. Also, > I'm not really sure I like it. If a user expects a marker to be there > and the marker isn't it feels silly not to give any feedback. I could see > a lot of users expecting ust to complain when you activate non-existent > markers, following the rule of least-suprise I think that it should > complain. > > Have you considered adding a parameter for this? Like a FORCE_ACTIVATE > flag or something? As we move to the new ltt-sessiond scheme with integration of the kernel and userspace LTTng / LTTng-UST, enabling tracepoints in not-yet-running applications and not-yet-loaded libraries will become an even more common use-case. I don't plan on spending too much time polishing the usttrace interface, given the changes planned. Thanks, Mathieu > > /Nils > >> Thanks, >> >> Mathieu >> >>> >>> /Nils >>> >>>> Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com> >>>> --- >>>> .../libustctl_function_tests.c | 10 ++++++---- >>>> 1 files changed, 6 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/tests/libustctl_function_tests/libustctl_function_tests.c b/tests/libustctl_function_tests/libustctl_function_tests.c >>>> index 947028f..7c12695 100644 >>>> --- a/tests/libustctl_function_tests/libustctl_function_tests.c >>>> +++ b/tests/libustctl_function_tests/libustctl_function_tests.c >>>> @@ -21,6 +21,7 @@ >>>> #include <unistd.h> >>>> #include <sys/types.h> >>>> #include <sys/wait.h> >>>> +#include <errno.h> >>>> >>>> #include <ust/marker.h> >>>> #include <ust/ustctl.h> >>>> @@ -136,15 +137,16 @@ static void ustctl_function_tests(pid_t pid) >>>> >>>> tap_ok(!ustctl_destroy_trace(trace, pid), "ustctl_destroy_trace - without ever starting"); >>>> >>>> + tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid) == 0, >>>> + "Enable non-existent marker ustl blar"); >>>> >>>> printf("##### Tests that definetly should work are completed #####\n"); >>>> printf("############## Start expected failure cases ##############\n"); >>>> >>>> tap_ok(ustctl_set_marker_state(trace, "ust","bar", 1, pid), >>>> "Enable already enabled marker ust/bar"); >>>> - >>>> - tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid), >>>> - "Enable non-existent marker ustl blar"); >>>> + tap_ok(EEXIST == errno, >>>> + "Right error code for enabling an already enabled marker"); >>>> >>>> tap_ok(ustctl_start_trace(trace, pid), >>>> "Start a non-existent trace"); >>>> @@ -161,7 +163,7 @@ int main(int argc, char **argv) >>>> int i, status; >>>> pid_t parent_pid, child_pid; >>>> >>>> - tap_plan(27); >>>> + tap_plan(28); >>>> >>>> printf("Function tests for ustctl\n"); >>>> >>>> -- >>>> 1.7.2.3 >>>> >>>> >>>> _______________________________________________ >>>> ltt-dev mailing list >>>> ltt-dev at lists.casi.polymtl.ca >>>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev >>>> >>> >>> _______________________________________________ >>> ltt-dev mailing list >>> ltt-dev at lists.casi.polymtl.ca >>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev >>> >> >> -- >> Mathieu Desnoyers >> Operating System Efficiency R&D Consultant >> EfficiOS Inc. >> http://www.efficios.com >> > -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <BLU0-SMTP5860CC88B2AC6EDE7D2AEE96DB0@phx.gbl>]
* [ltt-dev] [UST PATCH] Fix libustctl_function_tests [not found] ` <BLU0-SMTP5860CC88B2AC6EDE7D2AEE96DB0@phx.gbl> @ 2011-02-23 21:20 ` Nils Carlson 0 siblings, 0 replies; 8+ messages in thread From: Nils Carlson @ 2011-02-23 21:20 UTC (permalink / raw) On Wed, 23 Feb 2011, Mathieu Desnoyers wrote: > * Nils Carlson (nils.carlson at ericsson.com) wrote: >> On Wed, 23 Feb 2011, Mathieu Desnoyers wrote: >> >>> * Nils Carlson (nils.carlson at ericsson.com) wrote: >>>> On Wed, 23 Feb 2011, Yannick Brosseau wrote: >>>> >>>>> After discussions, we concluded that the enable a non existing marker is a valid case, so we >>>>> move it to the working case section. >>>>> While being there, check that the re-enable a marker set the right errno >>>>> >>>> >>>> I don't really understand your reasoning. This is a question of libust >>>> being broken and the test-case showing this. We should fix the test-case. >>>> It should not be possible to enable a non-existent marker, the function >>>> should return -1, not 0 as it currently does. >>> >>> Enabling a non-existent marker/tracepoint is a perfectly valid case. If >>> the application later on loads a library with dlopen that contains this >>> marker, it should be enabled as soon as the library is loaded. So this >>> requires that the user can ask for enabling markers that do not appear >>> in the marker list yet. I do that to handle module loading in LTTng >>> (kernel). >> >> Ok. So we can pre-activate a marker... Interesting. Then I agree we can >> change the test-case. Though maybe somebody should document this as it >> doesn't make a lot of sense unless you consider the library case. Also, >> I'm not really sure I like it. If a user expects a marker to be there >> and the marker isn't it feels silly not to give any feedback. I could see >> a lot of users expecting ust to complain when you activate non-existent >> markers, following the rule of least-suprise I think that it should >> complain. >> >> Have you considered adding a parameter for this? Like a FORCE_ACTIVATE >> flag or something? > > As we move to the new ltt-sessiond scheme with integration of the kernel > and userspace LTTng / LTTng-UST, enabling tracepoints in not-yet-running > applications and not-yet-loaded libraries will become an even more > common use-case. I don't plan on spending too much time polishing the > usttrace interface, given the changes planned. > Maybe we should discuss these plans? I have a feeling there might be some duplication of work going on. Also activating trace-points in not yet started applications sounds risky as today everything is handled by pid. /Nils > Thanks, > > Mathieu > >> >> /Nils >> >>> Thanks, >>> >>> Mathieu >>> >>>> >>>> /Nils >>>> >>>>> Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com> >>>>> --- >>>>> .../libustctl_function_tests.c | 10 ++++++---- >>>>> 1 files changed, 6 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/tests/libustctl_function_tests/libustctl_function_tests.c b/tests/libustctl_function_tests/libustctl_function_tests.c >>>>> index 947028f..7c12695 100644 >>>>> --- a/tests/libustctl_function_tests/libustctl_function_tests.c >>>>> +++ b/tests/libustctl_function_tests/libustctl_function_tests.c >>>>> @@ -21,6 +21,7 @@ >>>>> #include <unistd.h> >>>>> #include <sys/types.h> >>>>> #include <sys/wait.h> >>>>> +#include <errno.h> >>>>> >>>>> #include <ust/marker.h> >>>>> #include <ust/ustctl.h> >>>>> @@ -136,15 +137,16 @@ static void ustctl_function_tests(pid_t pid) >>>>> >>>>> tap_ok(!ustctl_destroy_trace(trace, pid), "ustctl_destroy_trace - without ever starting"); >>>>> >>>>> + tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid) == 0, >>>>> + "Enable non-existent marker ustl blar"); >>>>> >>>>> printf("##### Tests that definetly should work are completed #####\n"); >>>>> printf("############## Start expected failure cases ##############\n"); >>>>> >>>>> tap_ok(ustctl_set_marker_state(trace, "ust","bar", 1, pid), >>>>> "Enable already enabled marker ust/bar"); >>>>> - >>>>> - tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid), >>>>> - "Enable non-existent marker ustl blar"); >>>>> + tap_ok(EEXIST == errno, >>>>> + "Right error code for enabling an already enabled marker"); >>>>> >>>>> tap_ok(ustctl_start_trace(trace, pid), >>>>> "Start a non-existent trace"); >>>>> @@ -161,7 +163,7 @@ int main(int argc, char **argv) >>>>> int i, status; >>>>> pid_t parent_pid, child_pid; >>>>> >>>>> - tap_plan(27); >>>>> + tap_plan(28); >>>>> >>>>> printf("Function tests for ustctl\n"); >>>>> >>>>> -- >>>>> 1.7.2.3 >>>>> >>>>> >>>>> _______________________________________________ >>>>> ltt-dev mailing list >>>>> ltt-dev at lists.casi.polymtl.ca >>>>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev >>>>> >>>> >>>> _______________________________________________ >>>> ltt-dev mailing list >>>> ltt-dev at lists.casi.polymtl.ca >>>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev >>>> >>> >>> -- >>> Mathieu Desnoyers >>> Operating System Efficiency R&D Consultant >>> EfficiOS Inc. >>> http://www.efficios.com >>> >> > > -- > Mathieu Desnoyers > Operating System Efficiency R&D Consultant > EfficiOS Inc. > http://www.efficios.com > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [ltt-dev] [UST PATCH] Fix libustctl_function_tests 2011-02-23 17:46 [ltt-dev] [UST PATCH] Fix libustctl_function_tests Yannick Brosseau 2011-02-23 18:51 ` Nils Carlson @ 2011-02-24 8:56 ` Nils Carlson 1 sibling, 0 replies; 8+ messages in thread From: Nils Carlson @ 2011-02-24 8:56 UTC (permalink / raw) merged, added another patch explaning why this should work. /Nils On Wed, 23 Feb 2011, Yannick Brosseau wrote: > After discussions, we concluded that the enable a non existing marker is a valid case, so we > move it to the working case section. > While being there, check that the re-enable a marker set the right errno > > Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com> > --- > .../libustctl_function_tests.c | 10 ++++++---- > 1 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/tests/libustctl_function_tests/libustctl_function_tests.c b/tests/libustctl_function_tests/libustctl_function_tests.c > index 947028f..7c12695 100644 > --- a/tests/libustctl_function_tests/libustctl_function_tests.c > +++ b/tests/libustctl_function_tests/libustctl_function_tests.c > @@ -21,6 +21,7 @@ > #include <unistd.h> > #include <sys/types.h> > #include <sys/wait.h> > +#include <errno.h> > > #include <ust/marker.h> > #include <ust/ustctl.h> > @@ -136,15 +137,16 @@ static void ustctl_function_tests(pid_t pid) > > tap_ok(!ustctl_destroy_trace(trace, pid), "ustctl_destroy_trace - without ever starting"); > > + tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid) == 0, > + "Enable non-existent marker ustl blar"); > > printf("##### Tests that definetly should work are completed #####\n"); > printf("############## Start expected failure cases ##############\n"); > > tap_ok(ustctl_set_marker_state(trace, "ust","bar", 1, pid), > "Enable already enabled marker ust/bar"); > - > - tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid), > - "Enable non-existent marker ustl blar"); > + tap_ok(EEXIST == errno, > + "Right error code for enabling an already enabled marker"); > > tap_ok(ustctl_start_trace(trace, pid), > "Start a non-existent trace"); > @@ -161,7 +163,7 @@ int main(int argc, char **argv) > int i, status; > pid_t parent_pid, child_pid; > > - tap_plan(27); > + tap_plan(28); > > printf("Function tests for ustctl\n"); > > -- > 1.7.2.3 > > > _______________________________________________ > ltt-dev mailing list > ltt-dev at lists.casi.polymtl.ca > http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-02-24 8:56 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-23 17:46 [ltt-dev] [UST PATCH] Fix libustctl_function_tests Yannick Brosseau
2011-02-23 18:51 ` Nils Carlson
2011-02-23 18:43 ` Mathieu Desnoyers
2011-02-23 18:56 ` Nils Carlson
[not found] ` <BLU0-SMTP641C88D30CC26BE35858F96DB0@phx.gbl>
2011-02-23 20:54 ` Nils Carlson
2011-02-23 20:56 ` Mathieu Desnoyers
[not found] ` <BLU0-SMTP5860CC88B2AC6EDE7D2AEE96DB0@phx.gbl>
2011-02-23 21:20 ` Nils Carlson
2011-02-24 8:56 ` Nils Carlson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox