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

Use baseText= in TestText or switch to TestValueXX for short cases #5373

Draft
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

markcmiller86
Copy link
Member

@markcmiller86 markcmiller86 commented Jan 12, 2021

Description

This is follow on to #5370 to begin work of replace cases where TestText() is used with baseline files that contain a small amount of text which can easily by codified into the baseText= option of the call or where TestValueXX() methods might work better. So, this will change various .py test files and remove a slew of baseline .txt files...I am guessing on the order of 100.

But, is that worth it? There are a number of other observations I now have regarding testing and design of our tests that suggest its ripe for an overhaul. Maybe this is less important than that work?

Type of change

Feature...simplify some testing logic

How Has This Been Tested?

Manually on my macOS system.

Checklist:

  • Update these docs
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • ❌ I have updated the release notes
  • ❌ I have made corresponding changes to the documentation
  • ❌ I have added debugging support to my changes
  • ❌ I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • ❌ I have added any new baselines to the repo
  • I have assigned reviewers (see VisIt's PR procedures for more information).

brugger1
brugger1 previously approved these changes Jan 12, 2021
@markcmiller86 markcmiller86 changed the title Improve TestText using baseText= option for short cases Use baseText= in TestText or switch to TestValueXX for short cases Jan 13, 2021
@markcmiller86
Copy link
Member Author

@visit-dav/visit-developers can I please ask for people to have a look at this and see if it makes sense.

For many cases, the TestText(..., baseText=...) or wholesale replacement of TestText() with TestValueEQ() works fine but a little less so for situations where case names are programmtically generated. In particular, look at changes to line_scan.py file. Because the test case names are programmatically generated there, to hold baseline results, I created a python dictionary object to maintain the mapping between case names and baseline results.

The kind of work in this PR means several hundred of our simpler baselined text results can be removed and baseline values can be more conveniently updated directly in the .py files.

brugger1
brugger1 previously approved these changes Jan 15, 2021
@markcmiller86
Copy link
Member Author

Looking for TestText() results ripe for replacement with TestValueXX() (looks or last 90 shortest text files)

find ./test/baseline -name '*.txt' -exec wc -c {} \; | sort -n -r | \
tail -n 90 | cut -d' ' -f2 | xargs -n 1 -i sh -c "echo '{}'; cat '{}'; echo -e '\n'"
../../test/baseline/simulation/mesh/mesh00.txt
Simulation executable "mesh" exists.
Simulation "mesh" started.
VisIt connected to simulation "mesh".

../../test/baseline/simulation/life/life00.txt
Simulation executable "life" exists.
Simulation "life" started.
VisIt connected to simulation "life".

../../test/baseline/plugins/plotsVsInstall/plotsVsInstall.txt
{   'Contour': 'success',
    'Label': 'success',
    'Tensor': 'success',
    'Volume': 'success'}


../../test/baseline/databases/wave_tv/wave_tv_04.txt
With TreatAllDBsAsTimeVarying set to 1 (true),
AddPlot() returned 0 for state 16 and 1 for state 17

../../test/baseline/simulation/var/var00.txt
Simulation executable "var" exists.
Simulation "var" started.
VisIt connected to simulation "var".

../../test/baseline/simulation/csg/csg00.txt
Simulation executable "csg" exists.
Simulation "csg" started.
VisIt connected to simulation "csg".

../../test/baseline/simulation/amr/amr00.txt
Simulation executable "amr" exists.
Simulation "amr" started.
VisIt connected to simulation "amr".

../../test/baseline/databases/silo/silo_29.txt
Pseudocolor:  (InvalidVariableException)
viewer: An invalid variable (d_split) was specified.

../../test/baseline/queries/bestfitline/bestline_2_05.txt
The best fit line is: Y = 1.00005X + 0.000256063 with a correlation coefficient of: 0.962636

../../test/baseline/queries/conncomp/conncomp_3d_volume_t2.txt
Found 2 connected components
Component 0 Volume = (6.9914)
Component 1 Volume = (0.466155)


../../test/baseline/queries/conncomp/conncomp_3d_centroid_t3.txt
Found 1 connected component
Component 0 [6912 cells] Centroid = (1.27461,0.551184,1.25659)


../../test/baseline/hybrid/conn_cmfe/conn_cmfe_10.txt
Pseudocolor:  (ExpressionException)
viewer: An invalid variable (pressure) was specified.

../../test/baseline/databases/closedatabase/closedatabase_exprs01.txt
The database VISIT_TOP_DIR/data/silo_hdf5_test_data/globe.silo was closed.
Expressions:


../../test/baseline/queries/xrayimage/xrayimage29.txt
The x ray image query results were written to the files output00.png - output01.png


../../test/baseline/queries/xrayimage/xrayimage04.txt
The x ray image query results were written to the files output00.png - output01.png


../../test/baseline/queries/py_queries/py_queries_01.txt
Input Vars:
 d(found=True)
 p(found=True)
Input Arguments:
 test
 0.0
 [1, 2.0, 3]



../../test/baseline/hybrid/pos_cmfe/pos_cmfe_10.txt
Pseudocolor:  (ExpressionException)
viewer: An invalid variable (xyz) was specified.

../../test/baseline/hybrid/conn_cmfe/conn_cmfe_06.txt
Pseudocolor:  (ExpressionException)
viewer: The file "bad_file.silo" does not exist.

../../test/baseline/operators/displace/ops_disp08.txt
FilledBoundary:  ()
viewer: The displace operator requires vector plot data.


../../test/baseline/queries/bestfitline/bestline_2_03.txt
The best fit line is: Y = 0.33333X + 10 with a correlation coefficient of: 1

../../test/baseline/hybrid/conn_cmfe/conn_cmfe_13.txt
Pseudocolor:  (ExpressionException)
viewer: The file "30" does not exist.

../../test/baseline/operators/ex_surf/ops_ex_surf_02.txt
The actual number of zones is 6600.
The number of ghost zones is 1728.

../../test/baseline/queries/xrayimage/xrayimage26.txt
The x ray image query results were written to the file output00.png


../../test/baseline/queries/xrayimage/xrayimage24.txt
The x ray image query results were written to the file output00.png


../../test/baseline/queries/xrayimage/xrayimage22.txt
The x ray image query results were written to the file output00.png


../../test/baseline/queries/xrayimage/xrayimage20.txt
The x ray image query results were written to the file output00.png


../../test/baseline/queries/xrayimage/xrayimage18.txt
The x ray image query results were written to the file output00.png


../../test/baseline/queries/xrayimage/xrayimage16.txt
The x ray image query results were written to the file output00.png


../../test/baseline/queries/xrayimage/xrayimage14.txt
The x ray image query results were written to the file output00.png


../../test/baseline/queries/xrayimage/xrayimage12.txt
The x ray image query results were written to the file output00.png


../../test/baseline/queries/xrayimage/xrayimage10.txt
The x ray image query results were written to the file output00.png


../../test/baseline/queries/xrayimage/xrayimage08.txt
The x ray image query results were written to the file output00.png


../../test/baseline/queries/xrayimage/xrayimage06.txt
The x ray image query results were written to the file output00.png


../../test/baseline/queries/xrayimage/xrayimage01.txt
The x ray image query results were written to the file output00.png


../../test/baseline/queries/bestfitline/bestline_2_01.txt
The best fit line is: Y = 1X with a correlation coefficient of: 1

../../test/baseline/hybrid/pos_cmfe/pos_cmfe_07.txt
Pseudocolor:  ()
viewer: The file "bad_file.silo" does not exist.

../../test/baseline/simulation/globalids/globalids13.txt
Times:
0.314159265359
1.88495559215
3.45575191895
5.02654824574


../../test/baseline/simulation/globalids/globalids_1_12.txt
Times:
5.02654824574
5.02654824574
5.3407075111
5.65486677646


../../test/baseline/queries/l2norm/l2norm_05.txt
The standard deviation of the relative difference is 0.185888

../../test/baseline/queries/conncomp/conncomp_3d_volume_t3.txt
Found 1 connected component
Component 0 Volume = (7.45191)


../../test/baseline/queries/avg_value/avg_value_02.txt
The average value of vel is 4.06619e-17, -1.2443e-17, 0

../../test/baseline/queries/centroid/centroid_03.txt
Centroid = (-0.000140032, -5.30633e-05, 1.00538e-05)

../../test/baseline/simulation/zerocopy/scalable_parallel_icet/zerocopy02.txt
ERROR: Some methods used more memory than copying.


../../test/baseline/simulation/ucdcurve/ucdcurve05.txt
Times:
0.314159265359
1.57079632679
4.71238898038


../../test/baseline/plots/contour/contour_bad_value.txt
Plot Completed: True
Number of compute engines: 1


../../test/baseline/queries/curvature/curvature_03.txt
The average mean curvature (manual) is -1.971365


../../test/baseline/databases/reopen/reopen_7_01.txt
VisIt was *NOT* able to reopen reopen_globe.silo!

../../test/baseline/queries/centroid/centroid_05.txt
Centroid = (-2.27026e-05, 5.12131, -2.31896e-17)

../../test/baseline/queries/centroid/centroid_04.txt
Centroid = (0.49986, -5.30633e-05, 1.00538e-05)

../../test/baseline/simulation/zerocopy/parallel/zerocopy02.txt
OK: All methods use less memory than copying.


../../test/baseline/simulation/zerocopy/zerocopy02.txt
OK: All methods use less memory than copying.

../../test/baseline/queries/revolved_surface_area/revolved_surface_area_04.txt
The total RevolvedSurfaceArea is 4.91214 cm^2

../../test/baseline/queries/revolved_surface_area/revolved_surface_area_03.txt
The total RevolvedSurfaceArea is 51.6719 cm^2

../../test/baseline/queries/revolved_surface_area/revolved_surface_area_02.txt
The total RevolvedSurfaceArea is 4.91838 cm^2

../../test/baseline/hybrid/pos_cmfe/pos_cmfe_06.txt
An invalid variable (pressure) was specified.

../../test/baseline/queries/centroid/centroid_09.txt
Centroid = (0.5, -2.41661e-08, -3.28694e-18)

../../test/baseline/rendering/legends/legends_29.txt
Before: ('Plot0019',)
After: ('Plot0020',)


../../test/baseline/queries/l2norm/l2norm_04.txt
The maximum relative difference is 0.988058

../../test/baseline/queries/l2norm/l2norm_03.txt
The minimum relative difference is 0.487300

../../test/baseline/queries/curvature/curvature_04.txt
The average value of curvature is -1.97137

../../test/baseline/queries/avg_value/avg_value_04.txt
The average value of pressure is 0.0380639

../../test/baseline/queries/avg_value/avg_value_03.txt
The average value of vel2 is 1, 784.418, 0

../../test/baseline/queries/centroid/centroid_13.txt
Centroid = (0.5, 0.763268, 0.549859)

../../test/baseline/operators/ex_surf/ops_ex_surf_03.txt
The actual number of zones is 6600.

../../test/baseline/hybrid/sil/sil6.txt
Material 1 was correctly left off.


../../test/baseline/rendering/scalable/glyphedPlot_SRModeHistory.txt
force SR mode 0
window 1:SR is ON


../../test/baseline/queries/l2norm/l2norm_02.txt
The average difference is 0.662186

../../test/baseline/hybrid/movie/movie_0_00.txt
MPEG movie file size is plausible.

../../test/baseline/queries/avg_value/avg_value_01.txt
The average value of t is 784.418

../../test/baseline/queries/length/length_01.txt
The total Length is 13.7573 cm

../../test/baseline/queries/conncomp/conncomp_3d_count_t1.txt
Found 16 connected components


../../test/baseline/queries/conncomp/conncomp_2d_count.txt
Found 77 connected components


../../test/baseline/queries/conncomp/conncomp_1d_count.txt
Found 15 connected components


../../test/baseline/queries/py_queries/py_queries_00.txt
The average value = 0.468215


../../test/baseline/queries/conncomp/conncomp_3d_count_t2.txt
Found 2 connected components


../../test/baseline/queries/centroid/centroid_08.txt
Centroid = (0.5, 0.762782, 0)

../../test/baseline/queries/centroid/centroid_07.txt
Centroid = (0.5, 0.666667, 0)

../../test/baseline/queries/conncomp/conncomp_3d_count_t3.txt
Found 1 connected component


../../test/baseline/queries/length/length_02.txt
The total Length is 1.41421

../../test/baseline/unit/convert2to3/simple_2to3_result.txt
print('Old Style Print!')


../../test/baseline/queries/watertight/watertight_01.txt
The surface is watertight.

../../test/baseline/queries/centroid/centroid_11.txt
Centroid = (0.5, 0.5, 0.5)

../../test/baseline/unit/convert2to3/simple_2to3_input.txt
print 'Old Style Print!'


../../test/baseline/databases/empty_db/empty_01.txt
DBYieldedNoDataException


../../test/baseline/rendering/legends/legends_35.txt
('Plot0024', 'text_obj')

../../test/baseline/rendering/legends/legends_33.txt
('Plot0023', 'text_obj')

../../test/baseline/queries/l2norm/l2norm_01.txt
The L2Norm is 4.3589.

../../test/baseline/queries/pickarray/pickarray_08.txt
The cycle is 340.

../../test/baseline/unit/embedded/embedded1.txt
PASSED

../../test/baseline/queries/queriesOverTime/TimePickRangeDict_01.txt
None

@cyrush
Copy link
Member

cyrush commented Feb 11, 2021

One note for folks to keep in mind - Failure modes for using the new methods might not be exactly the same.

Concrete Example:

TextText(GetQueryOutputString())

vs

TestValueEQ(GetQueryOutputObject()["min"],0))

GetQueryOutputObject() could return "None" and there is an exception that would stop the rest of the tests from running.
With the prev setup, we would detect a failure due to an empty output string.

Just to be clear: This isn't a bad thing, but might confuse us when changing over.

I think we should promote extra checks for bad return values in our CLI.

@markcmiller86
Copy link
Member Author

FYI...

% ./bin/visit -cli -nowin
Running: cli -dv -nowin
Running: viewer -dv -nowin -noint -host 127.0.0.1 -port 5600
>>> GetQueryOutputValue()
0.0
>>> GetQueryOutputValue()['min']
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'float' object has no attribute '__getitem__'
>>> f={'foo':1}
>>> f
{'foo': 1}
>>> f['max']
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: 'max'

So, even if GetQueryOutputValue() returns a valid dict, if key is wrong/misspelled, still get a python exception. We should certainly document this as part of writing good tests. We could...

def GetQueryValue(key=None):
    if key is None:
        return GetQueryOutputValue()
    else:
        try:
           retval = GetQueryOutputObject()[key]
        except:
            retval = None
    return retval

@markcmiller86
Copy link
Member Author

Whoops, I should have tested GetQueryOutputObject() in the above, not GetQueryOutputValue(). Nonetheless, I think the example demonstrates the point @cyrush is making. We should avoid writing test code that can except out of the whole .py file test.

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.

3 participants