Skip to content

Add support for switching between Generic vs SAP OData implementation - #106

Merged
phanak-sap merged 3 commits into
SAP:masterfrom
AlapanGhosh:sap_vendor
Oct 8, 2021
Merged

Add support for switching between Generic vs SAP OData implementation#106
phanak-sap merged 3 commits into
SAP:masterfrom
AlapanGhosh:sap_vendor

Conversation

@AlapanGhosh

Copy link
Copy Markdown
Contributor

Solves #105

Added a new config SAP_VENDOR_ENABLED to switch between Generic and SAP implementation of OData v2
@AlapanGhosh
AlapanGhosh marked this pull request as draft October 4, 2021 10:50
DirectBuilder now has a parameter to accept sap_vendor_enabled boolean value. The Decimal and Time generators have support for vendor switching
@codecov-commenter

codecov-commenter commented Oct 5, 2021

Copy link
Copy Markdown

Codecov Report

Merging #106 (a21b8fa) into master (4edcbfe) will increase coverage by 0.23%.
The diff coverage is 82.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #106      +/-   ##
==========================================
+ Coverage   61.94%   62.17%   +0.23%     
==========================================
  Files          19       19              
  Lines        3605     3630      +25     
==========================================
+ Hits         2233     2257      +24     
- Misses       1372     1373       +1     
Impacted Files Coverage Δ
odfuzz/generators.py 69.95% <75.00%> (+1.77%) ⬆️
odfuzz/config.py 91.04% <100.00%> (+1.04%) ⬆️
odfuzz/constants.py 100.00% <100.00%> (ø)
odfuzz/entities.py 74.14% <100.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4edcbfe...a21b8fa. Read the comment docs.

Comment thread odfuzz/entities.py
class DirectBuilder:
"""A class for building and initializing all queryable entities with metadata passed in constructor."""
def __init__(self, metadata, restrictions,method):
def __init__(self, metadata, restrictions,method, sap_vendor_enabled = False):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am thinking more about not adding too specific parameter - like the boolean flag here - but entire configuration dictionary, to build the Config class from (line 117).

For the purpose of testing this change, I am OK with merging the PR as is, however created issue #107 addressing this concern.

Comment thread tests/test_generators.py
assert generated_decimal == '0.000113'


def test_time_for_sap_vendor_enabled():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

missing test for vendor_enabled equals false , same for EDM.Decimal generator. Agreed to add in another PR.

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