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

Broken in R2016b #19

Closed
amroamroamro opened this issue Dec 11, 2016 · 3 comments
Closed

Broken in R2016b #19

amroamroamro opened this issue Dec 11, 2016 · 3 comments

Comments

@amroamroamro
Copy link

amroamroamro commented Dec 11, 2016

I realize this package is targeted at older MATLAB releases before the introduction of the matlab.unittest framework, but I wanted to report that matlab-xunit doesn't work anymore in the latest R2016b.

The problem is with function-based unit-tests and the initTestSuite.m script. Unfortunately this hack of running a script from another function won't work to get access to its local functions.

Let me give a simplified example to illustrate the issue.
Consider the following file containing function-based tests implemented as sub-functions:

my_tests.m

function funcs = my_tests
    my_init;

function test_1
    assert(true);

function test_2
    assert(true);

Then consider this simplified version of the init script:

my_init.m

funcs = cell(2,1);
funcs{1} = str2func('test_1');
funcs{2} = str2func('test_2');

Running the file in R2016b we get:

>> f = my_tests
f =
  2×1 cell array
    @test_1
    @test_2
>> functions(f{1})
ans =
  struct with fields:

    function: 'test_1'
        type: 'simple'
        file: ''
>> feval(f{1})
Undefined function or variable 'test_1'. 

So you can see that the function handles returned by str2func are not actually bound to the subfunctions, thus calling them will fail.

To add one more problem, if you call nargout from inside the script (similar to initTestSuite.m), it will actually throw an error now in the latest version.

The only solution I can think of is to use localfunctions introduced in R2013b, which is what matlab.unittest framework relies on:

function funcs = my_tests
    if verLessThan('matlab','9.1')
        my_init;
    else
        funcs = localfunctions;
    end

Now I get this:

>> functions(f{1})
ans = 
  struct with fields:

     function: 'test_1'
         type: 'scopedfunction'
         file: 'C:\path\to\my_tests.m'
    parentage: {'test_1'  'my_tests'}

Of course for matlab-xunit, you'll have to use create a helper function to iterate through the cell array of function handles returned by localfunctions and create a test suite out of them (similar to the related functiontests function but specifically for this package tests), i.e something like this:

function test_suite = test_something
    if verLessThan('matlab','9.1')
        initTestSuite;
    else
        test_suite = initTestSuiteFromFcnHandles(localfunctions);
    end

where initTestSuiteFromFcnHandles will basically filter function handles (extract setup, teardown, and test functions based on their names), then create a TestSuite from those composed of FunctionHandleTestCase tests.

I'm not sure if this issue is restricted to R2016b, so it might be a good idea to test on R2016a and R2015b too, which is around the time MATLAB switched to the new LXE execution engine, which might explain why it broke matlab-xunit..

@amroamroamro
Copy link
Author

amroamroamro commented Dec 11, 2016

I thought about it some more, and there are other hacks that can be implemented as alternative to localfunctions:

  • return a string to be evaluated in the proper context. Example:

    function funcs = my_tests2
        funcs = eval(my_init2());
    
    function test_1
        assert(true);
    

    where this time my_init2.m is a proper function with something like:

    function cmd = my_init2()
        cmd = 'cellfun(@str2func, which(''-subfun'', mfilename()), ''Uniform'',false);';
    end
    
  • pass an anonymous function from the test function context containing a reference to str2func. Example:

    function funcs = my_tests3
        funcs = my_init3(@(s)str2func(s), mfilename());
    
    function test_1
        assert(true);
    

    where

    function funcs = my_init3(s2f, filename)
        funcs = cellfun(s2f, which('-subfun', filename), 'Uniform',false);
    end
    

    note: it has to be an anonymous function like @(s)str2func(s) and not a direct function handle as @str2func, otherwise the name resolution will be wrong and will not be bound to the subfunctions...

Again in matlab-xunit, the list of function handles returned would then be processed into a test suite by a helper function like initTestSuiteFromFcnHandles I mentioned before.

@amroamroamro
Copy link
Author

amroamroamro commented Dec 11, 2016

ok, so here is my final proposed solution:

src/initTestSuite2.m

function varargout = initTestSuite2(s2f, filename)
    %INITTESTSUITE2  Utility function used for subfunction-based tests
    %
    % This file is a function that is called at the top of M-files containing
    % subfunction-based tests.
    %
    % The top of a typical M-file using this function looks like this:
    %
    %     function test_suite = testFeatureA
    %         test_suite = initTestSuite2(@(s) str2func(s));
    %
    % See also: localfunctions, functiontests
    %

    if nargin < 2
        % get filename from call stack
        [ST,I] = dbstack('-completenames');
        filename = ST(I+1).file;
    end

    % get function handles to subfunctions
    [subFcn, subFcnNames] = my_localfunctions(filename, s2f);

    % build a test suite from function handles
    test_suite = my_functiontests(subFcn, subFcnNames, filename);

    if nargout > 0
        varargout{1} = test_suite;
    else
        test_suite.run();
    end
end

function [funcs, names] = my_localfunctions(filename, s2f)
    names = which('-subfun', filename);
    funcs = cellfun(s2f, names, 'UniformOutput',false);
end

function suite = my_functiontests(subFcn, subFcnNames, filename)
    % setup function
    ind = find(xunit.utils.isSetUpString(subFcnNames));
    if isempty(ind)
        setupFcn = [];
    elseif numel(ind) > 1
        error('initTestSuite2:tooManySetupFcns', ...
            'Found more than one setup subfunction.')
    else
        setupFcn = subFcn{ind(1)};
    end

    % teardown function
    ind = find(xunit.utils.isTearDownString(subFcnNames));
    if isempty(ind)
        teardownFcn = [];
    elseif numel(ind) > 1
        error('initTestSuite2:tooManyTeardownFcns', ...
            'Found more than one teardown subfunction.')
    else
        teardownFcn = subFcn{ind(1)};
    end

    % test functions
    ind = xunit.utils.isTestString(subFcnNames);
    testsFcn = subFcn(ind);

    % test suite
    [~,name] = fileparts(filename);
    suite = TestSuite();
    suite.Name = name;
    suite.Location = filename;
    for k = 1:numel(testsFcn)
        suite.add(FunctionHandleTestCase(testsFcn{k}, setupFcn, teardownFcn));
    end
end

The usage would be:

function test_suite = testAssertFalse
    test_suite = initTestSuite2(@(s) str2func(s));
    % or
    %test_suite = initTestSuite2(@(s) str2func(s), mfilename('fullpath'));

function testAssertFalseHappyCase
    assertFalse(false);

So it doesn't require much change to existing tests. Plus it should work in all MATLAB versions.

If interested, I can submit a pull request to add this function and switch all unit tests to use it instead of initTestSuite.m script.

@bauglir
Copy link
Contributor

bauglir commented Dec 16, 2016

Thanks for the report @amroamroamro! I was notified about the issue about a day before you submitted this and had been working on a fix as well.

I ran into some edge cases which took a bit longer to approximately resolve/properly document, but I've submitted my PR as #20, along with some other compatibility fixes for R2016b.

Our solutions were mostly the same, except the route I've taken is more towards proposing a future alternative that for now is backwards compatible using initTestSuite.

I was intrigued by your @(s) str2func(s) proposal, especially with respect to the remaining issue I had with getting a list of handles for packaged functions. Unfortunately, it didn't prove to be able to provide a solution for that case.

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

No branches or pull requests

2 participants