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

Update the test framework #52

Merged
merged 51 commits into from
Sep 5, 2024
Merged

Update the test framework #52

merged 51 commits into from
Sep 5, 2024

Conversation

juzen2003
Copy link
Collaborator

@juzen2003 juzen2003 commented Aug 27, 2024

Update the test framework so that it's easier to write and update tests.

  • Fixed PdsFile needs a different test framework #40
    • Modfiy the test framework to either compare results with golden copies or update the golden copies with the latest output (currently opus products and associated abspaths has been updated to use the new test framework). Here are the example commands:
      • Update all golden copies using '--update' param
        pytest pdsfile/pds3file/rules/* --update
        pytest pdsfile/pds4file/rules/* --update
        
        or
        ./run_tests_coverage.sh --update
      • Update all golden copies of VG_28xx
        pytest pdsfile/pds3file/rules/VG_28xx.py --update
      • Update golden copy of VG_28xx opus products test
        pytest pdsfile/pds3file/rules/VG_28xx.py::test_opus_products --update
  • Fixed Create a command line utility to make it easier to write rules #41
    • Create a standalone script to display more readable opus products output. By default it will display the opus products output in table format. Here are example commands:
      • Given multiple paths, could be abspath or logical path in either pds3 or pds4 (--paths), display a table with opus type in the first column and its corresponding pdsfile list in the second column:
        python3 utility/show_opus_products.py --path '/Users/yu-jenchang/Dropbox (SETI Institute)/Shared-OPUS/pdsdata/holdings/volumes/COCIRS_5xxx/COCIRS_5408/DATA/POIDATA/POI0408010000_FP1.LBL' volumes/COISS_1xxx/COISS_1001/data/1294561143_1295221348/W1294561202_1.IMG
        or
        python3 utility/show_opus_products.py --paths '/Users/yu-jenchang/Dropbox (SETI Institute)/Shared-OPUS/pdsdata/holdings/volumes/COCIRS_5xxx/COCIRS_5408/DATA/POIDATA/POI0408010000_FP1.LBL' bundles/uranus_occs_earthbased/uranus_occ_u103_eso_220cm/data/rings/u103_eso_220cm_2200nm_radius_beta_ingress_100m.xml
      • Given a path (--paths), display output in pprint format (--pprint), the result of pprint format is handy to diff with the golden copies::
        python3 utility/show_opus_products.py --paths volumes/COCIRS_5xxx/COCIRS_5408/DATA/POIDATA/POI0408010000_FP1.LBL --pprint
      • Display output in tradition dictionary format (--raw):
        python3 utility/show_opus_products.py --paths volumes/COCIRS_5xxx/COCIRS_5408/DATA/POIDATA/POI0408010000_FP1.LBL --raw
      • Display output in a narrow table with opus type in top row and its corresponding pdsfile list in the second row (this is for files with long names):
        python3 utility/show_opus_products.py --paths volumes/COCIRS_5xxx/COCIRS_5408/DATA/POIDATA/POI0408010000_FP1.LBL volumes/COISS_1xxx/COISS_1001/data/1294561143_1295221348/W1294561202_1.IMG
      • Display output of the given opus types in table format (``--opus-types). It will filter out the wrong opus type (xxx in the example) and display the correct ones only: python3 utility/show_opus_products.py --paths volumes/COCIRS_5xxx/COCIRS_5408/DATA/POIDATA/POI0408010000_FP1.LBL --opus-types browse_thumb cocirs_browse_saturn cocirs_ispm xxx`
      • If all the given opus types are wrong, it will print a valid list of opus types and exit the program:
        python3 utility/show_opus_products.py --paths volumes/COCIRS_5xxx/COCIRS_5408/DATA/POIDATA/POI0408010000_FP1.LBL --opus-types wrong1 wrong2
  • Fixed Create top-level versions of some PdsFile functions #42
    • Create these top level functions in PdsFile class to set variables in Pds3File & Pds4File subclasses
    use_shelves_only
    require_shelves
    set_logger
    set_easylogger
    
  • Fixed Code coverage shows potential bugs/missing items in tests #7
    • Make sure test suite coverage is 100% by doing these:
      • update .coveragerc by removing omit /tests/, this will show us the test suite coverage.
      • Add blackbox/whitebox tests
      • Fixed bugs in the test suite
      • Remove functions not used
      • Remove the code branch that will never be reach.
  • Fixed The return value of "from_path" doesn't match the comments in the function #5
    • For the case 'checksums/archives/whatever':
      • In _complete, make sure 'checksums-archives-.*' category is properly cached, please see the comment in _complete function (line 3446-3463 in pdsfile.py)
    • For the case that ends with '.targz', like 'COISS_2001.targz':
      • In from_path, if bundlename is not the key to the CACHE, we try to figure out the bundleset based on the basename, and search the CACHE again using the bundleset as the key. (line 4214-4246 in pdsfile.py)
    • For the case with /version, like 'COISS_0xxx/v1':
      • In from_path, use regex to detector the slash version string and replace it with underscore version (line 4032-4037 in pdsfile.py)
  • Status of other bugs:
    • PDS3 rules test are inconsistent #37: Clean up tests in the rules. Now each volset will run the same opus products and associated abspaths tests ( under pdsfile/pdsfile_test_helper.py). test_opus_id_to_primary_logical_path (mostly added by Mark) is run in each volset as well, but the function is specific to each volset. There is test_opus_products_count function run in three volset only (added by Mark), not sure why we need this since we already have opus products tests. Maybe we should remove them?
    • Waiting on shelf file updates: Check pickle file ordering #6: Some old pickle files like ones created in 2020 doesn't have sorted keys, for example: 'holdings/_infoshelf-archives-volumes/COISS_3xxx_info.pickle'
      For ones created in 2021, they will have sorted keys, for example: 'holdings/_infoshelf-archives-volumes/COISS_0xxx_info.pickle' and 'holdings/_infoshelf-archives-volumes/HSTJx_xxxx_info.pickle', we can remove the sorting at line 5085 of _get_shelf function in pdsfile.py if all the shelf files are generated after 2021.
      Example code block to read the keys of pickle files:
    import pickle
    shelf_path = '/Users/yu-jenchang/Dropbox (SETI Institute)/Shared-OPUS/pdsdata/holdings/_infoshelf-archives-volumes/HSTJx_xxxx_info.pickle'
    with open(shelf_path, 'rb') as f:
            shelf = pickle.load(f)
    print(shelf.keys())
    

- Add '--update' param in pytest command line to create the golden copy
  by using the current opus products output.
- Fixed the output of show_opus_products.py
that read and compare with golden copies of expected results. All
golden copies can be updated using '--update' param when calling pytest
or coverage.
'--udpate' is passed in, the script will update the opus products
golden copies.
the expected golden copies.
- Add '--update' param in pytest to update the golden copies based on
the current opus products output.
the subclasses: use_shelves_only, require_shelves, set_logger,
set_easylogger
either read or update the golden copy of the test results.
helpers that can be applied to both Pds3File and Pds4File testings to
pdsfile/pdsfile_test_helper.py.
of a category is in its own line in the file.
- "--logical-path": pass in logical path for pdsfile instantiation.
- "-pds3": a flag to instantiate a Pds3File instance
- "-pds4": a flag to instantiate a Pds4File instance
- "--pprint": a flag to display output using pprint
product of the same category. Now each product file of the same opus
type will be in different lines with no horizontal separator lines
between them.
the opus products output belong to the given opus types. If wrong opus
types are given, it will desplay warning.
@juzen2003
Copy link
Collaborator Author

8/29/24 update: (Top comment is updated)

  • Change all golden copies from .py to .txt and store contents using pprint so that they can be easily diff to compare results.
  • Add and update more options for show_opus_products.py, example commands are added in the top comments (issue 41 area).

Copy link
Collaborator

@rfrenchseti rfrenchseti left a comment

Choose a reason for hiding this comment

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

  • The automated tests aren't running because the branch isn't up-to-date with main
  • When I run the tests myself against either the OPUS Dropbox or the real holdings I get a bunch of failures.

@@ -0,0 +1 @@
['volumes/COCIRS_5xxx/COCIRS_5408/DATA/APODSPEC/SPEC0408010000_FP1.DAT', 'volumes/COCIRS_5xxx/COCIRS_5408/DATA/APODSPEC/SPEC0408010000_FP1.LBL', 'volumes/COCIRS_5xxx/COCIRS_5408/DATA/ISPMDATA/ISPM0408010000_FP1.TAB', 'volumes/COCIRS_5xxx/COCIRS_5408/DATA/ISPMDATA/ISPM0408010000_FP1.LBL', 'volumes/COCIRS_5xxx/COCIRS_5408/DATA/TARDATA/TAR0408010000_FP1.TAB', 'volumes/COCIRS_5xxx/COCIRS_5408/DATA/TARDATA/TAR0408010000_FP1.LBL', 'volumes/COCIRS_5xxx/COCIRS_5408/BROWSE/TARGETS/IMG0408010000_FP1.PNG', 'volumes/COCIRS_5xxx/COCIRS_5408/BROWSE/TARGETS/IMG0408010000_FP1.LBL', 'volumes/COCIRS_5xxx/COCIRS_5408/DATA/POIDATA/POI0408010000_FP1.TAB', 'volumes/COCIRS_5xxx/COCIRS_5408/DATA/POIDATA/POI0408010000_FP1.LBL', 'volumes/COCIRS_5xxx/COCIRS_5408/DATA/GEODATA/GEO0408010000_611.LBL', 'volumes/COCIRS_5xxx/COCIRS_5408/DATA/GEODATA/GEO0408010000_611.TAB', 'volumes/COCIRS_5xxx/COCIRS_5408/DATA/GEODATA/GEO0408010000_617.LBL', 'volumes/COCIRS_5xxx/COCIRS_5408/DATA/GEODATA/GEO0408010000_617.TAB', 'volumes/COCIRS_5xxx/COCIRS_5408/DATA/GEODATA/GEO0408010000_699.LBL', 'volumes/COCIRS_5xxx/COCIRS_5408/DATA/GEODATA/GEO0408010000_699.TAB', 'volumes/COCIRS_5xxx/COCIRS_5408/BROWSE/SATURN/POI0408010000_FP1.LBL', 'volumes/COCIRS_5xxx/COCIRS_5408/BROWSE/SATURN/POI0408010000_FP1.PNG']
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be pretty-printed?

@@ -0,0 +1 @@
{('Cassini CIRS', 10, 'cocirs_cube', 'Spectral Image Cube', True): ['volumes/COCIRS_0xxx/COCIRS_0406/DATA/CUBE/POINT_PERSPECTIVE/000IA_PRESOI001____RI____699_F4_038P.tar.gz', 'volumes/COCIRS_0xxx/COCIRS_0406/DATA/CUBE/POINT_PERSPECTIVE/000IA_PRESOI001____RI____699_F4_038P.LBL', 'volumes/COCIRS_0xxx_v3/COCIRS_0406/DATA/CUBE/POINT_PERSPECTIVE/000IA_PRESOI001____RI____699_F4_038P.tar.gz', 'volumes/COCIRS_0xxx_v3/COCIRS_0406/DATA/CUBE/POINT_PERSPECTIVE/000IA_PRESOI001____RI____699_F4_038P.LBL'], ('Cassini CIRS', 20, 'cocirs_extra', 'Extra Cube Preview Image', False): ['volumes/COCIRS_0xxx/COCIRS_0406/EXTRAS/CUBE_OVERVIEW/POINT_PERSPECTIVE/000IA_PRESOI001____RI____699_F4_038P.JPG', 'volumes/COCIRS_0xxx/COCIRS_0406/EXTRAS/CUBE_OVERVIEW/POINT_PERSPECTIVE/000IA_PRESOI001____RI____699_F4_038P.LBL', 'volumes/COCIRS_0xxx_v3/COCIRS_0406/EXTRAS/CUBE_OVERVIEW/POINT_PERSPECTIVE/000IA_PRESOI001____RI____699_F4_038P.JPG', 'volumes/COCIRS_0xxx_v3/COCIRS_0406/EXTRAS/CUBE_OVERVIEW/POINT_PERSPECTIVE/000IA_PRESOI001____RI____699_F4_038P.LBL'], ('browse', 40, 'browse_full', 'Browse Image (full)', True): ['previews/COCIRS_0xxx/COCIRS_0406/DATA/CUBE/POINT_PERSPECTIVE/000IA_PRESOI001____RI____699_F4_038P_full.jpg'], ('browse', 30, 'browse_medium', 'Browse Image (medium)', False): ['previews/COCIRS_0xxx/COCIRS_0406/DATA/CUBE/POINT_PERSPECTIVE/000IA_PRESOI001____RI____699_F4_038P_med.jpg'], ('browse', 20, 'browse_small', 'Browse Image (small)', False): ['previews/COCIRS_0xxx/COCIRS_0406/DATA/CUBE/POINT_PERSPECTIVE/000IA_PRESOI001____RI____699_F4_038P_small.jpg'], ('browse', 10, 'browse_thumb', 'Browse Image (thumbnail)', False): ['previews/COCIRS_0xxx/COCIRS_0406/DATA/CUBE/POINT_PERSPECTIVE/000IA_PRESOI001____RI____699_F4_038P_thumb.jpg'], ('metadata', 4, 'cube_index', 'Cube Index', False): ['metadata/COCIRS_0xxx/COCIRS_0406/COCIRS_0406_cube_point_index.tab', 'metadata/COCIRS_0xxx/COCIRS_0406/COCIRS_0406_cube_point_index.lbl'], ('metadata', 8, 'supplemental_index', 'Supplemental Index', False): ['metadata/COCIRS_0xxx/COCIRS_0406/COCIRS_0406_cube_point_supplemental_index.tab', 'metadata/COCIRS_0xxx/COCIRS_0406/COCIRS_0406_cube_point_supplemental_index.lbl'], ('Cassini CIRS', 700, 'cocirs_documentation', 'Documentation', False): ['documents/COCIRS_0xxx/Volume-SIS.pdf', 'documents/COCIRS_0xxx/Spectral-Cube-SIS.pdf', 'documents/COCIRS_0xxx/Flasar-etal-2004-SSR.pdf', 'documents/COCIRS_0xxx/FOV-Overview.pdf', 'documents/COCIRS_0xxx/Data-Product-SIS.pdf', 'documents/COCIRS_0xxx/Chan-etal-2015.pdf', 'documents/COCIRS_0xxx/Cassini-CIRS-Final-Report.pdf', 'documents/COCIRS_0xxx/Calibration-Equations.pdf', 'documents/COCIRS_0xxx/CIRS-Users-Guide.pdf', 'documents/COCIRS_0xxx/CIRS-Interference.pdf']}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be pretty-printed?

@@ -265,7 +265,7 @@
r'bundles/\1/uranus_occ_support/document/user_guide/*-user-guide.pdf',
r'bundles/\1/uranus_occ_support/document/user_guide/*-user-guide.xml',
r'bundles/\1/uranus_occ_support/document/user_guide/*.pro',
r'bundles/\1/uranus_occ_support/document/user_guide/*.py',
r'bundles/\1/uranus_occ_support/document/user_guide/*.txt',
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are no *.txt files there. *.py was correct. The Python programs are part of the documentation.


@pytest.mark.parametrize(
'input_path,category,expected',
[
('uranus_occs_earthbased/uranus_occ_u0_kao_91cm/data/atmosphere/u0_kao_91cm_734nm_counts-v-time_atmos_egress.xml',
('bundles/uranus_occs_earthbased/uranus_occ_u0_kao_91cm/data/atmosphere/ u0_kao_91cm_734nm_counts-v-time_atmos_egress.xml',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there supposed to be a space in the filename?

# Set parameters for both Pds3File and Pds4File
######################################################################################
@classmethod
def use_shelves_only(cls, pds3_status=True, pds4_status=True):
Copy link
Collaborator

Choose a reason for hiding this comment

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

For this and require_shelves the point is to set both subclasses to the same value. If there are two parameters, we might as well just call the two subclass functions directly.


subclasses = cls.__subclasses__()
for child_class in subclasses:
child_class.set_logger(pdslogger.EasyLogger())
Copy link
Collaborator

Choose a reason for hiding this comment

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

These should probably be set to the set EasyLogger.

Copy link
Collaborator

@rfrenchseti rfrenchseti left a comment

Choose a reason for hiding this comment

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

For show_opus_products:

  • Perhaps we don't need the separate abspath and logical path command line options? Just try one and then the other and use whichever doesn't throw an error.
  • At the same time, it would be nice to be able to specify more than one path on the command line and just have it cycle through them printing the opus products for each.
  • Probably no point in printing the big "OPUS PRODUCTS OUTPUT" header since it's obvious they are opus products.

only one status parameter to set both Pds3File and Pds4File attributes
to the same status.
absolute paths or logical paths and could be pds3 or pds4, and display
all their outputs. Also add an option to display output in narrow table
so that files with long name won't break the table in narrow screen.
in these files:
- Bug fixed in test_opus_id_to_primary_logical_path in rules/COCIRS_xxx.py
- Tests and comments added in DATA_SET_ID in rules/COUVIS_0xxx.py
- Tests added in OPUS_ID_TO_PRIMARY_LOGICAL_PATH in rules/COVIMS_0xxx.py
a bug in FILENAME_KEYLEN in rules/COISS_xxxx.py
@juzen2003
Copy link
Collaborator Author

9/4/24 update: (Top comment is updated)

  • Update based on github comments
  • Fixed issue 5 and 7
  • Update the status of issue 6

@rfrenchseti rfrenchseti merged commit fe5ed96 into main Sep 5, 2024
11 checks passed
@rfrenchseti rfrenchseti deleted the update_test_framework branch September 5, 2024 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants