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

Got rid of screen size logic from test #1793

Merged
merged 1 commit into from Jan 20, 2016
Merged

Conversation

chaosphere2112
Copy link
Contributor

Not entirely certain why I put it there in the first place; test should run perfectly fine with a window that's bigger than the monitor.

@chaosphere2112
Copy link
Contributor Author

Fixes #1792

@durack1
Copy link
Member

durack1 commented Jan 14, 2016

@chaosphere2112 probably had something to do with OS X 10.8.old or was it Windows 3.1😉

@chaosphere2112
Copy link
Contributor Author

@durack1
3822381

@doutriaux1
Copy link
Contributor

@chaosphere2112 the bots use to pass before anyway. Does it pass again on your ubuntu?

@@ -46,12 +46,6 @@ def test_canvas_size(c, size, via):
info = c.canvasinfo()
w, h = size

# Make sure size fits on screen bounds
screen_w, screen_h = c.backend.renWin.GetScreenSize()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think GetScreenSize either works or not in VTK... Never could figure out when it does or not, so it probably a combination of this + small screen

Copy link
Contributor

Choose a reason for hiding this comment

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

@doutriaux1 if you remember @dlonie pushed a fix for this and now it should work on all systems.

@durack1
Copy link
Member

durack1 commented Jan 14, 2016

@chaosphere2112 @doutriaux1 just as a comment, if you are depending on a vnc configuration to test things, then I'd be pretty skeptical that it's representative of a physical machine configuration.. vnc provides super quirky results with graphics, trying to plot things etc etc.. Particularly if it's in a background mode..

@aashish24
Copy link
Contributor

@chaosphere2112 changes looks good to me. Yes, I agree that the test was not that useful since the initial screen size could be some number depending on the X or the underlying window system.

@aashish24
Copy link
Contributor

@doutriaux1 I am merging this.

aashish24 added a commit that referenced this pull request Jan 20, 2016
Got rid of screen size logic from test
@aashish24 aashish24 merged commit 6061e0f into master Jan 20, 2016
@aashish24 aashish24 deleted the fix_init_size_open branch January 20, 2016 02:14
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

4 participants