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

tests can change the current working directory #117

Closed
cbm755 opened this issue Oct 31, 2015 · 6 comments
Closed

tests can change the current working directory #117

cbm755 opened this issue Oct 31, 2015 · 6 comments

Comments

@cbm755
Copy link
Collaborator

cbm755 commented Oct 31, 2015

For example, after doctest cd, we end up in ${HOME}/octave, because the example does cd ~/octave.

Perhaps we should reset the cwd after running tests. For now a workaround would be to mark such tests as +SKIP.

@catch22
Copy link
Collaborator

catch22 commented Nov 11, 2015

Hm yeah. If we go down that route then perhaps we should even chdir after each evalc, but perhaps this is excessive? ;)

It seems that the examples of cd and ls should probably be +SKIPed, though - are they marked up as doctests in Octave?

@oheim
Copy link
Collaborator

oheim commented Nov 12, 2015

@catch22, yes, the particular cd ~/octave is wrapped inside an @example block. Therefore it should be doctested.

I would not want to go down that road and chdir after each test because (1) the chdir might be required for subsequent example which shall be tested, (2) this complicates things and (3) there will be other side effects with similar problems and doctest should be consistent about all of them.

On a side note: I believe that the doctest switches will not be loved by many developers (there were some reactions on the Octave mailing list). In this case the @example block with the cd command could be changed and put into a @code or @command macro.

@catch22
Copy link
Collaborator

catch22 commented Nov 12, 2015

@oheim It is in general not a good idea that tests rely on peculiarities of the host system (e.g., the presence of certain files). Therefore, code like cd ~/octave or, worse, ls with a fixed desired output should not be run automatically, unless the environment is suitably prepared. From what you are saying this means that some other environment should be used instead of @example.

I agree with your second point (and misremembered the implementation in my previous comment): doctest strives to isolate the doctests of each target (i.e., function, method, etc.) from side effects incurred by other targets. Therefore, we should chdir in doctest_collect after each call to doctest_run.

I share the dislike for toggles and directives (see #127). Apart from SKIP and XFAIL, which I recall are already part of Octave's current testing infrastructure, is there a particular reason for other switches to be used in Octave's doctest? For me other ones typically arise when I work around Octave-Matlab incompatibilities, deal with optional packages, etc.

@oheim
Copy link
Collaborator

oheim commented Nov 12, 2015

Therefore, we should chdir in doctest_collect after each call to doctest_run.

Why is that needed in the first place? For testing of functions which are loaded from the current directory? Since the doctest_collect happens before the doctest_run: Would it be possible, to (1) addpath ., (2) doctest whatever and (3) undo step 1? Then the examples are free to change the current directory.

Apart from SKIP and XFAIL, which I recall are already part of Octave's current testing infrastructure, is there a particular reason for other switches to be used in Octave's doctest?

I could argue that you wouldn't even need XFAIL, because such documentation should either be SKIPed because it is not executable or corrected because it is wrong. Any counter examples?

I find the SKIP switch useful for control of individual, not executable examples.

The SKIP_IF switches are good for Octave-matlab incompatibilities or different OS behaviour. However if you have to use these, the incompatibilities are likely to affect a wide range of examples and then it becomes effortless to skip them all, because this spoils the idea of testing something. For example, in the interval package most examples don't work on Windows OS, because the documentation differs from the output due to missing UTF-8 support.

@catch22
Copy link
Collaborator

catch22 commented Nov 12, 2015

Why is that needed in the first place?

So that you don't end up in ~/octave after doctesting cd 😄

I don't fully understand your second question. Note that doctest_collect calls doctest_run since @cbm755's work on recursion etc. (I agree that we should rename it to something more sensible, will add an issue).

I could argue that you wouldn't even need XFAIL, because such documentation should either be SKIPed because it is not executable or corrected because it is wrong. Any counter examples?

They have a different role: SKIP is for skipping examples that do not make sense to be executed (the ls doctest would be a good example, or functionality that depends on whether a given package is installed). In contrast, XFAIL is for documenting things that should work but do not work currently due to bugs, so this adds to the documentation. In this case, the test is actually run, i.e., it is an error if a test marked as XFAIL unexpectedly succeeds! This is the desired behavior, since it tests your expectations and thereby keeps the doctest up to date.

Note that SKIP (as opposed to SKIP_IF/SKIP_UNLESS) is probably mostly needed in diary mode, since in texinfo you have the choice of using @code instead of @example, as we discussed above.

For example, in the interval package most examples don't work on Windows OS, because the documentation differs from the output due to missing UTF-8 support.

I have a somewhat similar problem in quantbox. The style in https://github.com/catch22/quantbox/blob/master/sdp/solve_sdp.m#L36 helps to avoid excessive SKIP'ing but I can see that this does not scale. Let's collect some more data points..

@cbm755
Copy link
Collaborator Author

cbm755 commented Nov 12, 2015

I could argue that you wouldn't even need XFAIL, because...

At least in test, xfail is used for stuff that should be fixed---the summary explicitly makes a plea to help Octave by fixing these. I use xfail in both test and doctest in this way.

Would it be possible, to (1) ...

Probably. I'd be fine with a addpath-based traversal if it works (at the time, I felt chdir was simpler for me to implement but all that code could be refactored...) Almost hate to mention this but addpath might break private/ testing, at least on matlab ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants