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

Unit tests for 22429 and apply same treatment to other money format functions #22442

Merged
merged 1 commit into from
Jan 16, 2022

Conversation

demeritcowboy
Copy link
Contributor

Overview

Unit tests for #22429

This will fail at the moment since the fix isn't upmerged to master yet, but that's good since it shows the issue.

@civibot
Copy link

civibot bot commented Jan 10, 2022

(Standard links)

@civibot civibot bot added the master label Jan 10, 2022
@seamuslee001
Copy link
Contributor

ok forward port has now been done, so confirmed it failed before forward port and now re-running

@seamuslee001
Copy link
Contributor

Jenkins re test this please

@colemanw
Copy link
Member

retest this please

@demeritcowboy
Copy link
Contributor Author

This test passes locally. Will try just rebasing.

@demeritcowboy
Copy link
Contributor Author

Oh it's the moneyNumber() function that's failing. That function doesn't seem to be used anywhere but maybe needs the same treatment as money().

@demeritcowboy demeritcowboy changed the title [NFC] Unit tests for 22429 Unit tests for 22429 and apply same treatment to other money format functions Jan 14, 2022
@demeritcowboy
Copy link
Contributor Author

Have updated and applied the same empty-ish checks to the other functions that the test exposed.

@seamuslee001
Copy link
Contributor

Looks good to me IMO

@demeritcowboy demeritcowboy merged commit f47517e into civicrm:master Jan 16, 2022
@demeritcowboy demeritcowboy deleted the tests-22429 branch January 16, 2022 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants