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

Improve testing by moving common code to testing module #1968

Merged
merged 16 commits into from May 20, 2016

Conversation

Projects
None yet
3 participants
@aashish24
Contributor

aashish24 commented May 13, 2016

No description provided.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 13, 2016

Not ready to be merged but any feedback is welcome. @doutriaux1 @sankhesh @danlipsa @chaosphere2112

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 13, 2016

The core idea is to provide a regression testing module that provide api that is needed for most of the vcs regression testing. This work reduces the testing code to 20 - 60% in most of the cases.

@aashish24 aashish24 force-pushed the improve_testing_somemore branch 8 times, most recently from d1906cb to 9204ea6 May 13, 2016

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 13, 2016

@aashish24 aashish24 force-pushed the improve_testing_somemore branch 2 times, most recently from c1cc6da to 73c3d4b May 13, 2016

@aashish24 aashish24 force-pushed the improve_testing_somemore branch from 73c3d4b to 5dd0a94 May 13, 2016

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented May 13, 2016

@aashish24 I like this so far, I just gave it a quick glance but I like the concept.

@aashish24 aashish24 force-pushed the improve_testing_somemore branch from f9e5051 to d9f6525 May 13, 2016

@aashish24 aashish24 force-pushed the improve_testing_somemore branch 3 times, most recently from fbab187 to a1f6064 May 13, 2016

@aashish24 aashish24 changed the title from [WIP] Improve testing somemore to Improve testing by moving common code to testing module May 13, 2016

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 13, 2016

thanks @doutriaux1. This branch is ready for review now.

@aashish24 aashish24 force-pushed the improve_testing_somemore branch from a1f6064 to a2ce6ac May 13, 2016

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 13, 2016

@danlipsa @doutriaux1 would it be possible to get it merged soon so that I don't need to keep upto date with master?

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented May 13, 2016

ok let em build this on my mac an run the test suite on it.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented May 13, 2016

@aashish24 sorry my mac is very very slow.... still building

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 18, 2016

@danlipsa I just saw test_vcs_monotonic_decreasing_yxvsx_default.png too. Let me have a look.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 18, 2016

@danlipsa I just looked. My change should not have caused it but I am looking for suggestions.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 18, 2016

@doutriaux1 vcs_click_info is failing because of dimension issues. I will push a fix.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented May 18, 2016

@aashish24 strange. I just run the test on my branch and it generates the (smaller) image without the dots. I would say to file an issue for this, if we think it is small enough to let it through.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 18, 2016

okay thanks @danlipsa I will file the issue.

aashish24 added some commits May 18, 2016

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 18, 2016

Ref: #1976

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 18, 2016

I am running the tests again, this time I think I should have it. It seems to me that in parallel build, I didn't get these errors (weird).

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 18, 2016

should be all good now crossing my finger

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 18, 2016

@danlipsa @doutriaux1 please try again.

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented May 18, 2016

 54 - test_vcs_read_old_scr_2 (Failed)
421 - test_vcs_click_info (Failed)
422 - test_vcs_click_info_mollweide_boxfill (Failed)
423 - test_vcs_click_info_meshfill (Failed)
424 - test_vcs_click_info_robinson_meshfill (Failed)
562 - diags_test_02 (Failed)
563 - diags_test_03 (Failed)
564 - diags_test_04 (Failed)
565 - diags_test_41 (Failed)
566 - diags_test_05 (Failed)
567 - diags_test_06 (Failed)
568 - diags_test_07 (Failed)
569 - diags_test_08 (Failed)
570 - diags_test_09 (Failed)
571 - diags_test_10 (Failed)
572 - diags_test_11 (Failed)
573 - diags_test_12 (Failed)
574 - diags_test_13 (Failed)
575 - diags_test_15 (Failed)

This is uvctest-data
[~/projects/uvcdat/build/uvcdat-testdata (improve_testing_somemore %=)]$ git log
commit 426733695eb93d08904a6be993496ff45ad58d52
Author: Aashish Chaudhary aashish.chaudhary@kitware.com
Date: Wed May 18 11:04:33 2016 -0400

Updated baseline to match the correct size
@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 18, 2016

test_vcs_click_info is failing because it has to be in the foreground and width and height are not set. I tried to set geometry afterwards but looks like I found a bug. I will try to pass the geometry in the init call and see if that produces consistent behavior.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 18, 2016

@danlipsa I made the size of click tests to a fixed size. Can you try again please?

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented May 18, 2016

@aashish24 Those pass now, but it seems that you changed something else. I have 114 tests that fail - the size of the generated image is different - unless I did something wrong. Are those passing on your machine?
Here are the first few:

  3 - testEsmfRegridRegion (Failed)
 42 - test_vcs_boxfill_polar (Failed)
 46 - test_vcs_fillarea_transparency (Failed)
 49 - test_vcs_meshfill_vertices (Failed)
 54 - test_vcs_read_old_scr_2 (Failed)
 92 - test_vcs_remove_marker_none_1d (Failed)
 95 - test_vcs_monotonic_decreasing_yxvsx_default (Failed)
 96 - test_vcs_taylor_template_ctl (Failed)
 97 - test_vcs_taylor_2quads (Failed)
 98 - test_vcs_gen_meshfill (Failed)
 99 - test_vcs_setcolormap (Failed)
100 - test_vcs_1d_in_boxfill (Failed)
101 - test_vcs_1d_missing (Failed)
103 - test_vcs_1D_datawc (Failed)
104 - test_vcs_1D_datawc_missing (Failed)
105 - test_vcs_wmo_marker (Failed)
106 - test_vcs_wmo_markers (Failed)
107 - test_vcs_iso_celine_part1 (Failed)
108 - test_vcs_iso_celine_part2 (Failed)
109 - test_vcs_markers (Failed)
113 - test_vcs_boxfill_decreasing_latitude (Failed)
114 - test_vcs_meshfill_no_wrapping (Failed)
116 - test_vcs_meshfill_zoom (Failed)
@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented May 18, 2016

@aashish24 aashish24 force-pushed the improve_testing_somemore branch from 21ded89 to a142008 May 19, 2016

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 19, 2016

@danlipsa @doutriaux1 my bad, i relied on something from vcs which wasn't quite right. I will push a fix for vcs in a separate branch. In the mean time if you checkout the current SHA you should see only these tests failing: https://open.cdash.org/viewTest.php?onlyfailed&buildid=4372388

This should be the SHA:
[chaudhary@einstein src.git (improve_testing_somemore)]$ git log -n1
commit 9c121a2
Author: Aashish Chaudhary aashish.chaudhary@kitware.com
Date: Wed May 18 21:19:46 2016 -0400

Fixed failing test because of incorrect capture window size

[chaudhary@einstein src.git (improve_testing_somemore)]$

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented May 19, 2016

@aashish24 Now it is good. These are the only failing tests.
54 - test_vcs_read_old_scr_2 (Failed)
562 - diags_test_02 (Failed)
563 - diags_test_03 (Failed)
564 - diags_test_04 (Failed)
565 - diags_test_41 (Failed)
566 - diags_test_05 (Failed)
567 - diags_test_06 (Failed)
568 - diags_test_07 (Failed)
569 - diags_test_08 (Failed)
570 - diags_test_09 (Failed)
571 - diags_test_10 (Failed)
572 - diags_test_11 (Failed)
573 - diags_test_12 (Failed)
574 - diags_test_13 (Failed)
575 - diags_test_15 (Failed)

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 19, 2016

thanks @danlipsa finally 😌 @doutriaux1 okay to merge?

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented May 19, 2016

sorry lots of fires this morning, let me run it on my mac just to be safe.

@doutriaux1

This comment has been minimized.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented May 19, 2016

@doutriaux1 looks like you have an older copy of regression.py. If you look at the code here; https://github.com/UV-CDAT/uvcdat/blob/9c121a20f2121a0fcca665123d50b27553f176a0/Packages/testing/regression.py you will see that init takes arguments. Can you make sure that you have latest code installed (and the baseline).

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented May 20, 2016

@doutriaux1 Lets merge this in, otherwise nothing can get merged - this has lots of changes! Seems it is ready.

@danlipsa danlipsa merged commit 3d52e6e into master May 20, 2016

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented May 20, 2016

failed on my mac yesterday, but I'm off (officially today) I trust you @danlipsa go ahead and merge

@doutriaux1 doutriaux1 deleted the improve_testing_somemore branch May 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment