search

redhat-developer / dotnet-regular-tests

.NET Core tests for .NET Bunny (RHEL & Fedora RPM tests)
MIT License
8 stars 13 forks source link

Make liblttng-ust_sys-sdt.h pass when lttng is not installed #361

Closed omajid closed 2 months ago

omajid commented 3 months ago

The lttng-ust dependency is technically optional. If .NET is installed and that dependency isn't, assume .NET is okay.

omajid commented 3 months ago

@nicrowe00 What do you think? lttng-ust is an optional dependency. Should we force it to be installed for the sake of this test?

nicrowe00 commented 3 months ago

@omajid I'm inclined to say yes, we should force-install it for the test, just to verify that everything is as expected with the dependency. I don't mind though if we decide not to force-install it as it is optional like you mentioned. It depends on how worried we are about optional dependencies.

tmds commented 2 months ago

The goal of the test is to verify that lttng is working, we shouldn't make it PASS the test when a required dependency is missing.

@omajid I don't know what triggered the PR. If there is an environment where lttng is not present, you can describe it using a trait (like no-lttng), and add it as skipWhen to the test.json. In that environment you can set the trait by using the --trait argument on the test runner.

omajid commented 2 months ago

The goal of the test is to verify that lttng is working

It isn't. The test is trying to verify that lttng-ust has support for systemtap.

Which, now that I think about it, is not something relevant for .NET at all. .NET doesn't care if lttng-ust has some specific feature or not, as long as it works for tracing. And this test doesn't verify whether lttng-ust can trace correctly. So whether this feature is present or not doesn't make a difference to .NET

I am tempted to just delete this test.

If anything, this should be a test in lttng-ust, not .NET.

tmds commented 2 months ago

I am tempted to just delete this test.

Sounds good to me.