Skip to content
This repository has been archived by the owner on Sep 23, 2024. It is now read-only.

Add additional binary types #83

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jlloyd-widen
Copy link

@jlloyd-widen jlloyd-widen commented Nov 18, 2021

This addresses #66

  • [ x] New feature (non-breaking change which adds functionality)

Checklist

  • [ x] Description above provides context of the change
  • I have added tests that prove my fix is effective or that my feature works
  • Unit tests for changes (not needed for documentation changes)
  • CI checks pass with my changes
  • [ x] Bumping version in setup.py is an individual PR and not mixed with feature or bugfix PRs
  • Commit message/PR title starts with [AP-NNNN] (if applicable. AP-NNNN = JIRA ID)
  • Branch name starts with AP-NNN (if applicable. AP-NNN = JIRA ID)
  • Commits follow "How to write a good git commit message"
  • [ x] Relevant documentation is updated including usage instructions

Josh Lloyd and others added 2 commits November 18, 2021 09:52
@Samira-El
Copy link
Contributor

Hi @jlloyd-widen , thanks for the contribution!

Can you please add unit and integration tests that cover these new types? thx

@Samira-El Samira-El added the enhancement New feature or request label Dec 6, 2021
@jlloyd-widen
Copy link
Author

Can you please add unit and integration tests that cover these new types? thx

@Samira-El I tried running unit and integrations test (didn't even try adding new ones) and I couldn't get them to pass successfully. The instructions in the readme did not work for me. nosetest didn't actually run any tests and pytest fails half the tests right from the get-go. Until I have a way to pass all existing tests, I don't see a point to adding new ones. Some guidance on how to run these tests successfully would be appreciated.

@Samira-El
Copy link
Contributor

Hey, not sure what you meant by instructions didn't work for you and nosetests didn't run any tests.

Please try running again with the same commands as the CI job.

@jlloyd-widen
Copy link
Author

@Samira-El I've updated my code to incorporate your most recent changes and I've added a few integration tests regarding the blob, and binary types. I also fixed a few of the other data type integration tests that appeared to be failing. let me know what you think.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants