Xcov19 / project-healthcare

Healthcare Infrastructure as a Service: backend api for UI services to integrate location-aware consultation and diagnostics facilities for patient centric services
https://health.xcov19.dev
GNU Lesser General Public License v2.1
3 stars 6 forks source link

[C4GT Community]: Raise NotImplementedError in InterfaceProtocolCheckMixin if either classes lack the declared method. #33

Open codecakes opened 1 month ago

codecakes commented 1 month ago

Ticket Contents

Description

InterfaceProtocolCheckMixin checks for correct signature used by the implementation class. You can drop in the mixin wherever an implementation is subclassed with an interface definition.

For instance, a service interface is defined here And It's implementation is here

InterfaceProtocolCheckMixin however doesn't exactly check if both the implementation and interface have the declared method.

Goals

Goals

Expected Outcome

During runtime:

  1. Implementing an interface with InterfaceProtocolCheckMixin that lacks a method defined in interface will raise NotImplementedError.
  2. Implementing an interface with InterfaceProtocolCheckMixin that has a method defined in interface but not in the implementation will raise NotImplementedError.

Acceptance Criteria

Implementation Details

Check if both classes have the method declared here: https://github.com/Xcov19/project-healthcare/blob/964861545c969258caa5e86dfc63e413bf545fca/xcov19/utils/mixins.py#L18

UPDATE If you have an Interface class BaseClass and an implementation class IncorrectImplementation :

class BaseClass:
    def method_with_params(self, param1: int, param2: str):
        pass

class IncorrectImplementation(BaseClass, InterfaceProtocolCheckMixin):
    def method2(self, param1: int):
        pass

Then, Running IncorrectImplementation should raise NotImplementedError because method method_with_params is missing from IncorrectImplementation and method2 is not defined in the Interface BaseClass

Mockups/Wireframes

No response

Product Name

project-healthcare

Organisation Name

XCoV19

Domain

Healthcare

Tech Skills Needed

Debugging, Python, Test, Testing Library

Mentor(s)

@codecakes

Complexity

Low

Category

Backend, Beginner Friendly, Testing

Blacksujit commented 3 weeks ago

@codecakes can you please assign this to me i am interested in working on this issue

MAVRICK-1 commented 3 weeks ago

@codecakes can you assign me this one

codecakes commented 3 weeks ago

@codecakes can you please assign this to me i am interested in working on this issue

@Blacksujit feel free to raise the PR. it will be automatically assigned to whoever has a PR in review.

codecakes commented 3 weeks ago

@codecakes can you assign me this one

@MAVRICK-1 feel free to raise the PR. it will be automatically assigned to whoever has a PR in review.

RISHIKESHk07 commented 2 weeks ago

@codecakes was giving this a try but got stuck in pytests for this , any guide i could follow , i tested it on already implemented check for params not matching but it shows no error raised (i am on "main" branch )

codecakes commented 2 weeks ago

@codecakes was giving this a try but got stuck in pytests for this , any guide i could follow , i tested it on already implemented check for params not matching but it shows no error raised (i am on "main" branch )

@RISHIKESHk07 share instructions on what steps you took to reproduce your issue including command, code and screenshot.

RISHIKESHk07 commented 2 weeks ago

@codecakes wrote a new test file and tested it , on main branch image

codecakes commented 2 weeks ago

@codecakes wrote a new test file and tested it , on main branch image

@RISHIKESHk07 Good effort to begin with but you are proving the point that you have already declared the method. You are not doing anything wrong but contradicting the issue this ticket asks to implement. The issue clearly states to check if the method, in your case method_with_params, is absent in either classes, then it should raise.

In your particular case, if you run this:


class BaseClass:
    def method_with_params(self, param1: int, param2: str):
        pass

class IncorrectImplementation(BaseClass, InterfaceProtocolCheckMixin):
    def method_with_params(self, param1: int):
        pass

IncorrectImplementation()

you will get a traceback like this because a.) you already have declared the method BUT b.) It has different signature.

Traceback (most recent call last):
    ...
    raise NotImplementedError(f"""Signature for {defined_method} not correct:
NotImplementedError: Signature for method_with_params not correct:
                Expected: ['self', 'param1', 'param2']
                Got: ['self', 'param1']

I suggest to read the description and goals again.

Additionally, when sharing a bug with a screenshot that you want others to try which has a reproducible code, share the code separately when sharing an issue on any ticket. I suggest to look at the following to understand what details to share as a helpful guide: image

RISHIKESHk07 commented 1 week ago

@codecakes Thanks for suggestions , i was not able get pytest working as shown in the terminal of photo ( figured it out after some trial and error ) . Will try to move forward in the issue

codecakes commented 1 week ago

@codecakes Thanks for suggestions , i was not able get pytest working as shown in the terminal of photo ( figured it out after some trial and error ) . Will try to move forward in the issue

@RISHIKESHk07 There is nothing wrong with your pytest setup. It is working and that is why it fails.

See contributing to understand how to run tests generally.

codecakes commented 1 week ago

@codecakes Thanks for suggestions , i was not able get pytest working as shown in the terminal of photo ( figured it out after some trial and error ) . Will try to move forward in the issue

@RISHIKESHk07 Check updated Implementation details section for clarity

RISHIKESHk07 commented 1 week ago

@codecakes this is what i came up with , any suggestion i could make it better image

codecakes commented 1 week ago

@RISHIKESHk07 I am unable to understand the context properly. I suggest you follow the guidelines provided here when sharing an issue. See here and If you get stuck to understand how to share an issue and your findings. See what happens when standards are not followed. This is what I see when I click on your screeshot: image

From whatever I can gather from your screenshot, once you have implemented the code and written a unit test for it, raise a PR

@codecakes this is what i came up with , any suggestion i could make it better image