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

Clean up unit tests for onestop-python-client #1500

Open
8 tasks done
kenhtanaka opened this issue Mar 22, 2021 · 10 comments
Open
8 tasks done

Clean up unit tests for onestop-python-client #1500

kenhtanaka opened this issue Mar 22, 2021 · 10 comments
Assignees

Comments

@kenhtanaka
Copy link
Member

kenhtanaka commented Mar 22, 2021

Summary

As a Developer
I want to be able to Change all constructor signatures to not take a config file path but parameters and improve unit tests for the onestop-python-client module
So that I can have better test coverage without errors and not pass a config file into every constructor.
  • Change classes so none have config files passed in, only parameters needed.
  • Updated effected documentation
  • Get unit tests working
  • Change unittests to not load config but have their own dict they pass in, no need for loading from a config file for unittests.
  • Move unit tests to onestop-python-client/tests/unit and put test_WebPublisher in onestop-python-client/tests/integration
  • Update Circle CI to run all tests, had to rename/move unit tests to test/unit/test*.py so python unittest discovery can find them. Moved integration test to test/integration
  • Set build environment variables for integration tests to use.
    - [ ] Update scripts to at least accurate, unsure if all can work. Moved to new story
  • Refactored S3Utils.connect so clearer, adds one more param but is clearer.
@erinreeves erinreeves changed the title Clean up unit tests for python-client Clean up unit tests for onestop-python-client Mar 23, 2021
@erinreeves erinreeves self-assigned this Mar 23, 2021
@erinreeves
Copy link
Contributor

erinreeves commented Mar 27, 2021

Questions:

  • Lots of merged but not deleted branches, so unsure if should look at any for if these fixes have been done.
  • Future story: Should we refactor out non-extractor code from Extractor class? Such as the connecting to S3?
  • Refactor Chris did puts the parsing of config files on whatever calls S3MessageAdapter, S3Utils, SqsConsumer. Reflected in current script s3_notification_handler. Verification ok?
  • Someone give me an example of executing python3 scripts/sqs-to-registry/s3_notification_handler.py?

@erinreeves
Copy link
Contributor

Also need find story about updating circleCI config to remove CLI build.

@erinreeves
Copy link
Contributor

Only have log_level in one config?

@erinreeves
Copy link
Contributor

Check if relevant for story 1233:
Follow upstory for S3Utils:

  1. Rename method def get_csv_s3(self, boto_client, bucket, key):
    https://github.com/cedardevs/onestop-clients/blob/master/onestop-python-client/onestop/util/S3Utils.py#L99
    so directly below we have read_bytes_s3 ...
    so maybe we have get_s3_bytes
    get_s3_file
    read_bytes_s3 -> get_s3_bytes
    get_csv_s3 -> get_s3_file

  2. For CsbExtractor, shouldn't be doing any connecting
    https://github.com/cedardevs/onestop-clients/blob/master/onestop-python-client/onestop/extract/CsbExtractor.py#L42

@erinreeves
Copy link
Contributor

erinreeves commented Apr 8, 2021

S3MessageAdapter:

  • look into transform method used to return im_message.file_format, headers, - gone now. why?

@erinreeves
Copy link
Contributor

erinreeves commented Apr 28, 2021

Classes todo modification:

  • S3Utils

  • CsbExtractor

  • WebPublisher

  • KafkaConsumer

  • KafkaPublisher

  • SqsConsumer

  • Change unittests to not load config but have their own dict they pass in, no need for loading from a config file for unittests.

@erinreeves
Copy link
Contributor

What is the incoming collection_uuid in KafkaPublisher class for publish_collection and publish_granule?

@erinreeves
Copy link
Contributor

Build passing!!

@erinreeves
Copy link
Contributor

Which scripts can be run locally versus have to be copied to a pod to be executed?

archive_client_integration.py
bucket_automation.py
launch_delete_handler.py
launch_e2e.py
launch_kafka_publisher.py
launch_pyconsumer.py

sme/
sme.py
smeFunc.py
spatial.py

sqs-to-registry/
s3_notification_handler.py

@erinreeves
Copy link
Contributor

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

No branches or pull requests

2 participants