| msg212343 - (view) |
Author: Saimadhav Heblikar (Saimadhav.Heblikar) * |
Date: 2014-02-27 09:29 |
| This patch does 1.Remove pep8 violations in PathBrowser.py . Replaces "file","dir","sorted" by "file_","dir_","sorted_" respectively. 2.Extends test coverage for PathBrowser.py in idle_test/test_PathBrowser.py New modules now under tests include 1.dirBrowserTreeItem - getText,ispackagedir 2.pathBrowserTreeItem - getText,getSublist Only method missing a test after this patch will be listmodules.I am not too sure whether it requires a test for itself.it is indirectly being tested from lines 8 and 9(in the current tip). |
|
|
| msg212348 - (view) |
Author: Steven D'Aprano (steven.daprano) *  |
Date: 2014-02-27 10:22 |
| I think you may have misread PEP 8. It does not recommend a trailing underscore for names that shadow built-ins (e.g. file_ instead of file), it only recommends a trailing underscore when you need to use a keyword as a name (e.g. class_ instead of class). Shadowing built-ins should be done with care, but is permitted. If there is little or no risk of confusion with the built-ins, there is no need to worry about shadowing them. It's just another name. |
|
|
| msg212363 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2014-02-27 16:26 |
| We do not usually make pure style changes in a file unless the code is being reviewed and edited. Even then, I focus on changes that make the file easier to read and understand, as is necessary to write tests. One example is adding missing spaces after commas in function definitions and calls (not a problem in this file, as far as I see). Another is to add PEP8 style docstrings to document the behavior being tested. That is an issue for this file. The name clashes do not bother me: 'file' is no longer a builtin in 3.x; 'sorted' was used in the file before the builtin was added (though I might think of something different if I were writing today). So lets leave the names alone and focus on good docstrings. |
|
|
| msg212399 - (view) |
Author: Saimadhav Heblikar (Saimadhav.Heblikar) * |
Date: 2014-02-28 00:14 |
| Well,thank you for the feedback to both . I will try to make a new patch , during this weekend, removing the name changes to add docstrings and add a human testable dialog for pathbrowser,as is present in other tests. |
|
|
| msg212435 - (view) |
Author: Saimadhav Heblikar (Saimadhav.Heblikar) * |
Date: 2014-02-28 14:24 |
| test_DirBrowserTreeItem (idlelib.idle_test.test_pathbrowser.PathBrowserTest) ... ok test_PathBrowserTreeItem (idlelib.idle_test.test_pathbrowser.PathBrowserTest) ... ok ---------------------------------------------------------------------- Ran 2 tests in 0.008s OK As suggested in and ,i have removed the proposed changes to variable names (see ) Added tests as described in Added a working test dialog for visual testing the PathBrowserDialog.(This feature was not working as far as i know) |
|
|
| msg247003 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2015-07-20 21:46 |
| New changeset 61d7e6fe0003 by Terry Jan Reedy in branch '2.7': Issue #20792: Expand idle_test.test_pathbowser. Tweak file. https://hg.python.org/cpython/rev/61d7e6fe0003 New changeset 0220328f962c by Terry Jan Reedy in branch '3.4': Issue #20792: Expand idle_test.test_pathbowser. Tweak file to not copy twice. https://hg.python.org/cpython/rev/0220328f962c |
|
|
| msg247004 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2015-07-20 21:49 |
| I only did the test_pathbrowser changes now. I added an assert that failed in 2.7 because Idle still defines some old-style classes not subclassing object. The 'main' test had been rewriten as an htest. Am leaving issue open to look at those changes another time. |
|
|
| msg265660 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2016-05-16 02:40 |
| The Feb 2014 patches contained changes to Pathbrowser.main. In June/July 2014, module 'main' tests were converted to htests. I have determined that the htest supercedes the main changes and that this issue can be closed. |
|
|