plone / guillotina

Python AsyncIO data API to manage billions of resources

https://guillotina.readthedocs.io/en/latest/

Other

187 stars 51 forks source link

[5.x] Fixed validation of query params that are integers #936

Closed lferran closed 4 years ago

lferran commented 4 years ago

@masipcat any idea why this test is failing? it has nothing to do with my changes as far as I understand :thinking:

masipcat commented 4 years ago

@lferran it seems the test started failed in this commit https://github.com/plone/guillotina/commit/1212c6fd0dbbe0252b1287e680e29be338f1c924

The problem is here: https://github.com/plone/guillotina/commit/1212c6fd0dbbe0252b1287e680e29be338f1c924#diff-b5c749b5b1f60f743b66487afd3aa472R21

should be super().__init__(value, type, field_name)

lferran commented 4 years ago

ouch, it still fails.

vangheem commented 4 years ago

hmmm, I don't understand how this has happened multiple times now that something was reporting that it passed...

vangheem commented 4 years ago

@lferran I pushed fix to your branch.

masipcat commented 4 years ago

I think there's a ghost...

lferran commented 4 years ago

I think it the ghost was because of this https://github.com/plone/guillotina/pull/936/commits/039cda8cbaa3555950c01ff6937e6d9d6dbcf1b8

I got all tests running now without having to modify the super().__init__(value)

does it make sense at all?

masipcat commented 4 years ago

Yes, because @vangheem changed the test https://github.com/plone/guillotina/pull/936/commits/9f9e8d017c9ae781ebac4fbbefc288d3f341f423 , right?

codecov-io commented 4 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (5.x@670cde7). Click here to learn what that means. The diff coverage is 100%.

@@          Coverage Diff          @@
##             5.x    #936   +/-   ##
=====================================
  Coverage       ?   94.6%           
=====================================
  Files          ?     306           
  Lines          ?   27911           
  Branches       ?       0           
=====================================
  Hits           ?   26393           
  Misses         ?    1518           
  Partials       ?       0

Impacted Files	Coverage Δ
guillotina/test_package.py	`96.7% <ø> (ø)`
guillotina/component/_api.py	`97.3% <100%> (ø)`
guillotina/tests/test_swagger.py	`100% <100%> (ø)`
guillotina/api/service.py	`93.3% <100%> (ø)`
guillotina/schema/tests/test__field.py	`99.3% <100%> (ø)`
guillotina/schema/exceptions.py	`100% <100%> (ø)`

lferran commented 4 years ago

@masipcat so you are suggesting to change back to

super().__init__(value, type, ...)

and undo nathan's changes on the test, aren't you?