There is a overlooked issue with the line that sets the amount of the payment. It was probably meaning to cast the result of the calculation to a string, but instead casts the value being devided to a string, resulting in an InvalidArgumentException if there is a decimal amount.

Patch coming up

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

eiriksm created an issue. See original summary.

eiriksm’s picture

Status: Active » Needs review
FileSize
897 bytes

Patch attached. Will create a follow up to create a test, since that should be fairly simple to create.

zaporylie’s picture

I refactored the class a bit to make it easier for write a test, which I also wrote. I found two more places where parenthesis were missing but I haven't had time to write tests for those as tests would have to be functional. That test coverage can be added later.

Status: Needs review » Needs work

The last submitted patch, 3: 3064776-3.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

zaporylie’s picture

Status: Needs work » Needs review
FileSize
8.09 KB

Fixes the test namespace. Hopefully that will make tests to work in DrupalCI environment.

Status: Needs review » Needs work

The last submitted patch, 5: 3064776-5.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

zaporylie’s picture

Status: Needs work » Needs review
FileSize
8.39 KB

Tests were discovered! (damn typo)

Now seems like the test_dependency is missing.

Status: Needs review » Needs work

The last submitted patch, 7: 3064776-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

zaporylie’s picture

Status: Needs work » Needs review
FileSize
8.75 KB

As per https://www.drupal.org/docs/develop/using-composer/managing-dependencies... seems like test_dependencies must be committed to default branch before they are discovered by DrupalCi, hence I'm adding relevant test dependencies to require-dev section of composer.json.

  • zaporylie authored 94edce4 on 8.x-4.x
    Issue #3064776 by zaporylie, eiriksm: createShipment can send floats to...
zaporylie’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.