Fix failing VariablesCommandTest for MariaDB 10.1.19 #255

Merged
merged 1 commit into from Dec 3, 2016

Conversation

Projects
None yet
3 participants
@jthln
Contributor

jthln commented Dec 3, 2016

Magerun pull-request check-list:

  • Pull request against develop branch (if not, just close and create a new one against it)
  • README.md reflects changes (if any)

Subject: Fix failing VariablesCommandTest for MariaDB 10.1.19

Changes proposed in this pull request:

  • Fixes failing VariablesCommandTest for MariaDB 10.1.19
1) N98\Magento\Command\Database\VariablesCommandTest::testRounding
Failed asserting that '"Variable Name",Value
aria_block_size,"  8.00K"
[---snip---]
myisam_max_sort_file_size," 10.00G"
[---snip---]
wsrep_max_ws_size,"  2.00G"
' matches PCRE pattern "~myisam_max_sort_file_size,"  [0-9\.]+[A-Z]"~".
@ktomk

This comment has been minimized.

Show comment
Hide comment
@ktomk

ktomk Dec 3, 2016

Contributor

Thanks for this PR. Do you think this is specific to Mariadb? I can imagine this is just some padding.

Contributor

ktomk commented Dec 3, 2016

Thanks for this PR. Do you think this is specific to Mariadb? I can imagine this is just some padding.

@jthln

This comment has been minimized.

Show comment
Hide comment
@jthln

jthln Dec 3, 2016

Contributor

Could be MariaDB specific, or maybe even my.cnf specific (didn't check though).

Contributor

jthln commented Dec 3, 2016

Could be MariaDB specific, or maybe even my.cnf specific (didn't check though).

@ktomk

This comment has been minimized.

Show comment
Hide comment
@ktomk

ktomk Dec 3, 2016

Contributor

Okay, then this is padding. The fairly large value from myisam_max_sort_file_size is causing the smaller ones to have an additional space to the left.

Fix looks good btw, waiting for the build to finish and will merge then.

Contributor

ktomk commented Dec 3, 2016

Okay, then this is padding. The fairly large value from myisam_max_sort_file_size is causing the smaller ones to have an additional space to the left.

Fix looks good btw, waiting for the build to finish and will merge then.

@jthln

This comment has been minimized.

Show comment
Hide comment
@jthln

jthln Dec 3, 2016

Contributor

Checked the configuration file, only one space there, so it seems to be MariaDB specific.

myisam_max_sort_file_size = 10G
Contributor

jthln commented Dec 3, 2016

Checked the configuration file, only one space there, so it seems to be MariaDB specific.

myisam_max_sort_file_size = 10G
@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Dec 3, 2016

Current coverage is 51.31% (diff: 100%)

Merging #255 into develop will not change coverage

@@            develop       #255   diff @@
==========================================
  Files           195        195          
  Lines          8609       8609          
  Methods         865        865          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           4418       4418          
  Misses         4191       4191          
  Partials          0          0          

Powered by Codecov. Last update 6cdda21...baf8be3

Current coverage is 51.31% (diff: 100%)

Merging #255 into develop will not change coverage

@@            develop       #255   diff @@
==========================================
  Files           195        195          
  Lines          8609       8609          
  Methods         865        865          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           4418       4418          
  Misses         4191       4191          
  Partials          0          0          

Powered by Codecov. Last update 6cdda21...baf8be3

@ktomk

This comment has been minimized.

Show comment
Hide comment
@ktomk

ktomk Dec 3, 2016

Contributor

No, you misread me sort of. 10G results in a string "10.00G" and all strings are prefixed with at least one space, so the string finally is " 10.00G", which is of size 7. Other values aren't two digits but one digit. As all values will be output with same length, the get the additional space at the left. It's not Mariadb specific, set that setting to 10G in a Mysql server and you can provoke the same failures. Hope this clarified it a bit.

Contributor

ktomk commented Dec 3, 2016

No, you misread me sort of. 10G results in a string "10.00G" and all strings are prefixed with at least one space, so the string finally is " 10.00G", which is of size 7. Other values aren't two digits but one digit. As all values will be output with same length, the get the additional space at the left. It's not Mariadb specific, set that setting to 10G in a Mysql server and you can provoke the same failures. Hope this clarified it a bit.

@ktomk ktomk merged commit b202501 into netz98:develop Dec 3, 2016

4 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 6cdda21...baf8be3
Details
codecov/project 51.31% (+0.00%) compared to 6cdda21
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ktomk

This comment has been minimized.

Show comment
Hide comment
@ktomk

ktomk Dec 3, 2016

Contributor

We believe that the bug reported is fixed in the development version. It can be upgraded to it using the self-update command with the --unstable switch.

Contributor

ktomk commented Dec 3, 2016

We believe that the bug reported is fixed in the development version. It can be upgraded to it using the self-update command with the --unstable switch.

@jthln

This comment has been minimized.

Show comment
Hide comment
@jthln

jthln Dec 3, 2016

Contributor

Other values aren't two digits but one digit.

Ah, I see 👍

Contributor

jthln commented Dec 3, 2016

Other values aren't two digits but one digit.

Ah, I see 👍

@jthln jthln deleted the jthln:feature/fix-variables-command-test branch Dec 3, 2016

cmuench added a commit to cmuench/n98-magerun2 that referenced this pull request Oct 11, 2017

Fix regex in VariablesCommandTest, closes #255
Fix failing VariablesCommandTest for MariaDB 10.1.19 (#255)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment