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

PageArrayTable: multiple values for 'arg_str' #1929

Merged
merged 5 commits into from Jul 6, 2016

Conversation

ScottWales
Copy link
Contributor

This patch fixes an error in the PageArrayTable widget, which fails when it attempts to display an array variable (i.e. a variable with the length metadata set). This error can be seen in the 01-types app of the demo-meta example suite.

I've included an example that can be run with py.test. Without the patch, running the test produces the error

    def make_row_valuewidget(self, *args, **kwargs):
        kwargs.update({"arg_str": str(self.length)})
>       return row.RowArrayValueWidget(*args, **kwargs)
E       TypeError: __init__() got multiple values for keyword argument 'arg_str'

lib/python/rose/config_editor/variable.py:554: TypeError

The error message is also displayed in the status bar of the GUI when using the widget to display arrays.

Scott Wales added 4 commits June 30, 2016 13:58
Using an unnamed argument creates a conflict in
'make_row_valuewidget()', where 'arg_str' is added to **kwargs
@matthewrmshin matthewrmshin added this to the next-release milestone Jun 30, 2016
@matthewrmshin
Copy link
Member

@benfitzpatrick please review and/or re-assign.

@benfitzpatrick
Copy link
Contributor

benfitzpatrick commented Jul 1, 2016

Thanks Scott!

Could you add your name to CONTRIBUTING.md as part of the PR?

The tests are nice - do you have time to quickly plug them into our test framework by moving them under a t/rose-config-edit/ directory?

You'd need to create that directory and make it look like the others, with a test_header symlink and e.g. an 00-rowwidget-ok.t file with e.g. pass_ok pytest rowwidget/test.py. An example directory would be t/rose-help/, but there are lots of others.

It would be best to have a skip over the test if pytest is not installed - t/rosie-disco/00-basic.t has an example of how to do that.

Use pytest-tap to output in TAP format
@ScottWales
Copy link
Contributor Author

Done. I was getting errors that rose couldn't be found when using the header in 00-pytest.t, so I've just copied that one function. I've also added a check for the pytest TAP plugin so that it outputs directly in TAP format. If you prefer to modify this to not use a plugin please feel free.

I'm already in the contributors file :)

Cheers, Scott

$ bash 00-pytest.t 
# TAP results for test_PageTable
ok 1 - test_PageTable
# TAP results for test_PageArrayTable
ok 2 - test_PageArrayTable
# TAP results for test_PageLatentTable
ok 3 - test_PageLatentTable
1..3

Without pytest-tap

$ bash 00-pytest.t 
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: No module named tap
1..0 # SKIP Python package "pytest-tap" not installed

@benfitzpatrick
Copy link
Contributor

Unfortunately, we don't have pytest installed, so I can't test it - but it is a good change!

@benfitzpatrick
Copy link
Contributor

@matthewrmshin please sanity check.

@matthewrmshin
Copy link
Member

Looks sane.

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

Successfully merging this pull request may close these issues.

None yet

3 participants