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

Improve testing and fix issue with 'generic' entities #17

Closed
wants to merge 4 commits into from

Conversation

gilesknap
Copy link
Member

@MaxHerbs we seem to have introduced a bug that changes an entity's module name to "generic" unless the module has a specific handler.

I figure that is my bad for not having any proper tests.

So I've added some tests and fixed the issue.

But would appreciate you having a glance to see if what I have done makes sense for your generic handler.

b2dfac3 is the fix

Thanks

Copy link

codecov bot commented Dec 15, 2024

Codecov Report

Attention: Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Project coverage is 84.32%. Comparing base (ee891b1) to head (b2dfac3).

Files with missing lines Patch % Lines
src/builder2ibek/__main__.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #17      +/-   ##
==========================================
+ Coverage   80.40%   84.32%   +3.92%     
==========================================
  Files          22       22              
  Lines         398      402       +4     
==========================================
+ Hits          320      339      +19     
+ Misses         78       63      -15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MaxHerbs
Copy link
Contributor

The fix looks good to me. I see where the bug was coming from.
I suspect most of the entities in my testing were covered by existing handlers, and I had been too focused on checking the global entities had been removed to spot any change of module name for the remaining entities.

The tests look far more thorough as well
Thanks

@gilesknap
Copy link
Member Author

Thanks @MaxHerbs. I'm going to close this as it has CI errors and is part of my upcoming 'autosave' branch merge anyway.

@gilesknap gilesknap closed this Dec 17, 2024
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.

2 participants