Feature request: testing private functions in packages #1017
Comments
|
Right now, you would need to write a small wrapper stylesheet like this where you expose some components in the included package. In that example, you can test the private |
|
@jcronk |
|
@AirQuick I haven't - in general, if a package has substantial enough private components that they need their own tests, that's a signal to me that I should separate that out into its own package with its own testable public API. A while back you mentioned maybe adding something like an |
|
I’m working with a package that exposes 3 public (final) functions (same name, different arity), which rely on 16 private "helper" functions. The private functions are not used or needed anywhere except as helpers for these three public functions, so putting them in the same package as the public ones seemed optimally legible and maintainable. Each of the helper functions provides a small, specific type of functionality, which made it easy to develop the code incrementally, and to refactor individual helper functions without having to touch anything else in the pipeline. It isn’t so much that the package has “has substantial enough private components that they need their own tests” as that small, specialized pipeline components, which in this case are private because they are only used as helper functions for the broader, public ones, seem like natural candidates for unit testing. As I wrote when I opened this issue (with apologies for garbling my Xs and Yx and As and Bs), if a public function breaks because of an error in one of the helper functions, unit testing at the level of the private, helper functions could make it easier to find and repair the error. Perhaps this is wrong-headed. It feels natural to me, but I'm not a software engineer, and my understanding of Best Practice may well be misguided. I also don’t understand what it would mean to put the private functions into their own package with their own testable public AP (see above). If the issue is that it isn’t currently possible to test private functions using XSpec, what might the testable public API look like, and why would that be preferable to being able to run regular XSpec tests over private functions? I don’t have a strong opinion about whether it would be preferable to declare that private functions should be tested for the test suite as a whole (at the |
|
I don't know, I think we're all still figuring this out, since the package concept is new to XSLT 3.0. That said, I think that there are some good reasons to only test the public API of a package - this way you're testing the visible behavior and not any of the implementation details, and that means that your tests don't necessarily have to change if the underlying implementation of your visible behavior changes. Wherever possible, I want to avoid spending any more time than I need to on writing or maintaining tests, and so I try to avoid situations where refactoring the parts of the code that I'm using will require rewriting the tests. For me, what I've been doing is looking at it in terms of what I want to be able to test. If there's a component that should be private to my package, but I think its behavior is important enough that it needs testing, then to me that suggests that the component is a public part of an API that my package depends on, making it into its own package. So far, for my use cases, this has worked, but if I run into a situation where this is overly arduous or it doesn't make sense, then I'm not opposed to looking for a different solution. |
|
@jcronk Thanks, Jeremy, for the response. As I wrote earlier, I'm not a software engineer, so perhaps I've simply misunderstood the purpose of testing, but I'll try to summarize my reasoning below, and if it is misguided, I'd be grateful for an explanation of why I shouldn't want to be able to do what I think I want to be able to do.
I agree, then, that we “want to avoid spending any more time than [we] need to on writing or maintaining tests, and so [we] try to avoid situations where refactoring the parts of the code that [we’re] using will require rewriting the tests”. But making it possible, but not required, to write tests for private functions meets that need because a developer who doesn’t want to test private functions 1) will not have to rewrite tests for the private functions because there aren’t any such tests, and 2) that same developer will not have to rewrite tests for the public functions that use the private ones because the visible behavior should not be impacted by implementation details. Perhaps my rationale for deciding when to divide functionality over multiple packages and when to keep it in a single package is wrong-headed, and in the interest of learning whether I have misunderstood how such decisions are made, I can share that I am guided by two considerations:
I am aware that I am the only participant in this conversation who is not a trained software developer, so if my reasoning is fallacious, I would appreciate learning what I have misunderstood. As I reread the discussion, though, I don’t understand how my proposal that it be possible to test private functions requires writing or rewriting extra tests, since the ability to test private functions does not confer a requirement to do so. |
|
Does this method work for you in some cases, or not work at all?
|
|
@AirQuick The wrapper does not seem to work. Here is my version (filename xspec-wrapper.xsl): <?xml version="1.0" encoding="UTF-8"?>
<xsl:package exclude-result-prefixes="#all" version="3.0" xmlns:djb="http://www.obdurodon.org"
xmlns:f="http://www.obdurodon.org/function-variables"
xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
<xsl:include href="plot-lib.xsl"/>
<xsl:expose component="*" names="djb:*" visibility="final"/>
</xsl:package>plot-lib.xsl is a package (root element <?xml version="1.0" encoding="UTF-8"?>
<x:description xmlns:x="http://www.jenitennison.com/xslt/xspec"
xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:djb="http://www.obdurodon.org"
xmlns:f="http://www.obdurodon.org/function-variables" stylesheet="xspec-wrapper.xsl"
run-as="external">I did not yet write any tests for private functions. When I try to run XSpec against xspec-wrapper.xspec , it apparently fails to recognize the included XSLT file as XSLT: |
|
@djbpitt The current XSpec implementation does not support testing private functions. But that's not because it's a bad practice. The lack of support is purely due to technical limitations. If there is a clean lightweight way to forcibly access private functions, I would like to utilize it for testing them. Unfortunately, I'm not aware of it. |
|
Thank you, @AirQuick, for the follow-up. I’m not sure what qualifies as clean or lightweight, and I don’t know much about how XSpec works under the hood, but since you mention being at a loss to identify a way to test private functions, I wonder whether the following might suggest a way forward: Could the testing process create shadow copies of packages, designate all functions in the copy as public, and then test the no-longer-private functions by testing the copy? Private functions might call on other private functions in the same package, but treating them all as public won’t interfere with those calls, and private functions cannot be called from outside their own package, so the only changes needed to test a package thoroughly would be to the private functions in the package being tested at the moment (and not in other packages it might use). I could, I suppose, transforming the package explicitly and then test the transformed version, but doesn’t seem either clean or lightweight, and an alternative that lets the user test the package directly, with the copying happening in the background, would maintain consistency in the testing process by not requiring the user to pre-process packages explicitly and externally before testing them. I fear that I probably don’t have the developer skills to implement this improvement myself, but if it seems like a constructive way forward, I would be eager to help in whatever way I can, whether by testing, or by helping, though these conversations, to frame the problem and identify a path toward a solution. |
|
Thanks @djbpitt for sharing the idea. I'm afraid that creating a temporary copy of the package would be costly and should generally be considered as the last resort. What if you define the private visibility attributes as shadow attributes and what if the static parameters are supported by XSpec? <xsl:package ...>
<xsl:param name="internal:internal-component-visibility"
as="xs:string"
select="'private'"
static="yes" />
<xsl:function name="f:complex-number"
_visibility="{$internal:internal-component-visibility}"><x:description run-as="external"
stylesheet=".../package.xsl"
...>
<x:param name="internal:internal-component-visibility"
select="'public'"
static="yes" />
<x:scenario label="Calling a private function in the package for testing purposes">
<x:call function="f:complex-number">This seems to work on my end. (Note that This way, the private functions are no longer private in its strict sense, but I thought it might possibly be an acceptable compromise. |
|
@AirQuick Thank you for this creative idea. If you’d like to implement support of Looking forward, though, I would normally want to weigh the costs and benefits of competing solutions, and not identify any of them as a last resort because of a cost until I had assessed the costs of the alternatives. In the interest of making that comparison explicit, it seems as if using a static parameter might entail the following costs, although they are costs in developer effort, and not in computation:
I appreciate that the static parameter might, after weighing the costs and benefits of all of the options (those above and those I may not have thought of), turn out to be the best compromise, and of course I’ll trust your judgment about the best way to meet the need for testing private functions. That the static parameter approach can be met with one-time costs (in each package, and in a wrapper to set the overriding value when running tests) does seem, when I consider the issues above, superior to an efficiency cost that is paid on every run, and that cannot be converted to a one-time cost. But I thought I should mention that the fact that copying comes with a cost in efficiency does not mean that the static parameter approach does not also come with a cost, and that it might be useful to compare those costs explicitly, as I’ve tried to do here. In any case, as I write above, I’ll be happy to test the static parameter approach once it has been implemented, and if it works as expected and you consider it the best available solution, we could close the issue. |
|
@djbpitt I opened #1120. Can you try it and let us know what you find out in it? Your possible use case is demonstrated in Another thought... I guess the most transparent way in your case would be to have a custom XML parser and plug it into Saxon. The parser would set |
|
@AirQuick Thank you! I’m at (that is, “at”) Balisage intensively this week, but I’ll check #1120 out over the weekend and report back. Plugging a custom XML parser into Saxon is beyond my ability, but along those same lines I could, in a shell script, transform my real library (using XSLT) to one that changes the visibility of the functions, creating my own shadow copy, and then run the tests against that. |
First, a big thank you to the developers for implementing support for testing packages.
#762 (comment) observes that private functions within packages cannot be tested, and discussed why this is both desirable and not desirable. Specifically:
Insofar as private functions are called by public or otherwise exposed components within a package, the private functions may be the lowest level of functionality, and therefore, it would seem, be sensible candidates for unit testing. For example, if public A calls private X and private Y and A fails a test, the locus of the failure could be in A itself or in B or C. Without being able to test B and C independently, we would seem to sacrifice one of the benefits of unit testing, that is, isolating a small unit of functionality. I understand why testing private components is challenging, but I don’t understand why it wouldn’t be beneficial for the developer of the package.
I’m not a Java developer, and perhaps that’s why the reason for this is not clear to me. I would agree that we should not treat public and private functions equivalently, since the developer of the package distinguished between them for a reason, and the inaccessibility of a private function outside its package is part of what it is. But if, for example, we decide to require an attribute on the scenario for testing a private function that identifies it as private (so that private functions would continue to be hidden unless the test acknowledged explicitly that they were private, but needed nonetheless to be tested), why would it be a Bad Thing if the testing process would create accessible shadows of private functions for testing purposes?
The text was updated successfully, but these errors were encountered: