[DTrace-devel] [PATCH 6/6] procfs: populate pr_argc, pr_argv, and pr_envp with default values

[Kris Van Hees]

Good point - I'll withdraw the patch.  We can discuss about the best way
to handle the test issues in a later patch (series).

On Thu, Jan 30, 2025 at 06:43:17PM -0500, Eugene Loh wrote:
> On 1/30/25 18:03, Kris Van Hees wrote:
> 
> > On Thu, Jan 30, 2025 at 05:33:45PM -0500, Eugene Loh wrote:
> > > My main question is how it is better to provide incorrect results than it is
> > > not to have implemented this functionality?  How about just waiting until we
> > > implement this functionality?  Isn't it better that a user is told that we
> > > haven't implemented this stuff rather than give them results that are
> > > incorrect?
> > That is always a balance.  Ideally, we should have tests for each independent
> > member of each translator, so that we can do more fine grained testing.  But
> > it is not sensible that we do not end up testing any translator members because
> > some are not implemented, whereas providing a default value for them (which we
> > e.g. also do for members in the io translators).  That is the precedent I am
> > depending on here.
> 
> From the user's point of view, the choice is between:
> 
> *)  the status quo, where an attempt to use one of the unimplemented fields
> returns a rather nice error message that indicates what the problem is
> 
> *)  the proposed patch, where we "silently" return incorrect values, leaving
> the user with incorrect results or wasting time debugging the problem,
> filing a bug, etc.
> 
> So, there is hardly a balance here at all for users:  the status quo is far
> better.
> 
> From our test suite's point of view, there is a choice between less coverage
> (some tests XFAIL) and "more coverage", but I question that characterization
> since the "more coverage" case still doesn't check the correctness of the
> output.  Anyhow, to the extent that this is a test-suite issue, let's fix it
> in the test suite instead of polluting user-visible behavior.  Let's not
> "game the test suite" by modifying dtrace.  The whole set of tst.psinfo*.d
> tests are in need of overhaul;  so let's improve test coverage by cleaning
> up the tests rather than feeding users incorrect results.
> 
> If you'd like me to take a stab at cleaning up tst.psinfo*.d, let me know.
> 
> > > It is odd that XFAIL is being lifted when we are producing incorrect output,
> > > but I suppose that is the burden of some other test/s.  So, okay.
> > Yes, we need more fine grained tests.
> 
> FWIW, test/unittest/builtinvar/tst.psinfo.d would also XPASS with this
> patch.
> 
> > > On 1/28/25 01:31, Kris Van Hees wrote:
> > > > The pr_argc, pr_argv, and pr_envp fields in psinfo are not implemented
> > > > yet, so it makes sense to set them to 0 rather than not providing any
> > > > translator for them.