Skip to content

fix(query): check valid warn message (#9919)#9948

Merged
sushantdhiman merged 1 commit into
sequelize:masterfrom
tsasaki609:iterate-warning-row
Sep 29, 2018
Merged

fix(query): check valid warn message (#9919)#9948
sushantdhiman merged 1 commit into
sequelize:masterfrom
tsasaki609:iterate-warning-row

Conversation

@tsasaki609

Copy link
Copy Markdown
Contributor

Pull Request check-list

Please make sure to review and check all of these items:

  • Does npm run test or npm run test-DIALECT pass with this change (including linting)?
  • Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
  • Have you added new tests to prevent regressions?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description of change

fix #9919
validate that warning row is iterable

@codecov

codecov Bot commented Sep 22, 2018

Copy link
Copy Markdown

Codecov Report

Merging #9948 into master will decrease coverage by 4.83%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #9948      +/-   ##
=========================================
- Coverage   96.14%   91.3%   -4.84%     
=========================================
  Files          63      63              
  Lines        9407    9408       +1     
=========================================
- Hits         9044    8590     -454     
- Misses        363     818     +455
Impacted Files Coverage Δ
lib/dialects/mysql/query.js 20.15% <100%> (-70.47%) ⬇️
lib/dialects/mysql/query-generator.js 4% <0%> (-93.78%) ⬇️
lib/dialects/mysql/query-interface.js 30.43% <0%> (-69.57%) ⬇️
lib/dialects/mysql/connection-manager.js 26.08% <0%> (-63.77%) ⬇️
lib/dialects/mysql/data-types.js 53.21% <0%> (-43.12%) ⬇️
lib/sql-string.js 83.82% <0%> (-7.36%) ⬇️
...dialects/abstract/query-generator/helpers/quote.js 88.88% <0%> (-5.56%) ⬇️
lib/sequelize.js 93.47% <0%> (-3.11%) ⬇️
lib/dialects/abstract/query-generator.js 96.33% <0%> (-1.31%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa2f373...668072c. Read the comment docs.

@sushantdhiman

Copy link
Copy Markdown
Contributor

Test case is included in that issue, please use that or create a new one

@tsasaki609

Copy link
Copy Markdown
Contributor Author

Oops I forgot. I made a simple unit test.
Thanks for the review.

Comment thread lib/dialects/mysql/query.js Outdated
const warningMessage = 'MySQL Warnings (' + (this.connection.uuid||'default') + '): ';
const messages = [];
for (const _warningRow of warningResults) {
if (typeof _warningRow[Symbol.iterator] !== 'function') break;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continue instead of break

Comment thread test/unit/dialects/mysql/query.test.js
@tsasaki609 tsasaki609 force-pushed the iterate-warning-row branch 4 times, most recently from 4e950af to a564919 Compare September 29, 2018 02:35
@tsasaki609

Copy link
Copy Markdown
Contributor Author

I implemented an integration test.

@sushantdhiman sushantdhiman merged commit d76b0a8 into sequelize:master Sep 29, 2018
@sushantdhiman

Copy link
Copy Markdown
Contributor

Thanks @tsasaki609

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants