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

PR: Add test for file directory change in the Project Explorer. #6413

Merged
merged 8 commits into from
Feb 15, 2018

Conversation

dalthviz
Copy link
Member

@dalthviz dalthviz commented Feb 8, 2018

Fixes #5601

@dalthviz dalthviz added this to the v3.2.7 milestone Feb 8, 2018
@dalthviz dalthviz self-assigned this Feb 8, 2018
@dalthviz dalthviz requested a review from ccordoba12 February 8, 2018 03:26
@ccordoba12 ccordoba12 changed the title Add test for file directory change in the Project Explorer. PR: Add test for file directory change in the Project Explorer. Feb 8, 2018
projects = main_window.projects

# Create a temp project directory
project_dir = tempfile.mkdtemp()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change this to use the tmpdir fixture instead.

@dalthviz dalthviz force-pushed the fixes_issue_5601 branch 3 times, most recently from 3a98abc to a158168 Compare February 9, 2018 04:48
projects.treewidget.setCurrentIndex(idx)

# Set a timer to manipulate the select directory dialog while it's running
QTimer.singleShot(2000, lambda: open_file_in_editor(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this timer is what's making the test to hang on Appveyor.

if isinstance(w, QFileDialog):
if directory is not None:
w.setDirectory(directory)
QTest.keyClick(w, Qt.Key_Enter)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This auxiliary function is not needed anymore. Please remove it.

@@ -205,6 +217,40 @@ def test_calltip(main_window, qtbot):
main_window.editor.close_file()


@pytest.mark.slow
@flaky(max_runs=3)
def test_change_directory_in_project_explorer(main_window, qtbot, tmpdir):
Copy link
Member

@ccordoba12 ccordoba12 Feb 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test doesn't need to interact with other plugins (e.g. the IPython console), so it doesn't belong here. Please move it to spyder/plugins/tests/test_projects.py.

@@ -15,12 +19,38 @@
from spyder.widgets.projects.explorer import ProjectExplorerTest

@pytest.fixture
def setup_projects_explorer(qtbot):
def setup_projects_explorer(qtbot, directory=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change this name to setup_project_explorer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a second thought, let's rename the fixture to project_explorer (instead of setup_project_explorer).

@@ -15,12 +19,38 @@
from spyder.widgets.projects.explorer import ProjectExplorerTest

@pytest.fixture
def setup_projects_explorer(qtbot):
def setup_projects_explorer(qtbot, directory=None):
"""Set up ProjectExplorerWidgetTest."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change this docstring to

Setup Project Explorer widget

qtbot.addWidget(project_explorer)
return project_explorer


def test_change_directory_in_project_explorer(qtbot, tmpdir):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to pass the fixture as an argument to the test, else it can't be initialized correctly.

@dalthviz dalthviz force-pushed the fixes_issue_5601 branch 5 times, most recently from 98a101d to 5423e4b Compare February 13, 2018 00:03
@@ -169,6 +169,7 @@ def delete(self, fnames=None):

class ProjectExplorerWidget(QWidget):
"""Project Explorer"""
redirect_stdio = Signal(bool)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this signal necessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -261,13 +262,16 @@ def delete_project(self):
# Tests
#==============================================================================
class ProjectExplorerTest(QWidget):
def __init__(self):
def __init__(self, directory=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make directory an attribute of this class, i.e.

self.directory = directory 

self.explorer = ProjectExplorerWidget(None, show_all=True)
self.explorer.setup_project(osp.dirname(osp.abspath(__file__)))
self.explorer = ProjectExplorerWidget(parent=self, show_all=True)
if directory:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if direcotory: -> if self.directory is not None:

self.explorer.setup_project(osp.dirname(osp.abspath(__file__)))
self.explorer = ProjectExplorerWidget(parent=self, show_all=True)
if directory:
self.explorer.setup_project(directory)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

directory -> self.directory

"""Setup Project Explorer widget."""
directory = request.node.get_marker('change_directory')
if directory:
project_dir = str(tmpdir.mkdir('project'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change str here for to_text_string

qtbot.addWidget(project_explorer)
return project_explorer
return (project_explorer, project_dir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make return only project_explorer here.

def test_change_directory_in_project_explorer(project_explorer, qtbot):
"""Test changing a file from directory in the Project explorer."""
# Create project
project, project_dir = project_explorer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be now

project = project_explorer
project_dir = project.directory

project_explorer.resize(250, 480)
project_explorer.show()
assert project_explorer
project_explorer_dlg, project_dir = project_explorer
Copy link
Member

@ccordoba12 ccordoba12 Feb 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be now

project = project_explorer

and please change below project_explorer_dlg to project.

@ccordoba12 ccordoba12 merged commit 2156327 into spyder-ide:3.x Feb 15, 2018
ccordoba12 added a commit that referenced this pull request Feb 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants