Skip to content

test: add async test case - #339

Merged
popomore merged 1 commit into
masterfrom
add-async-test
Feb 9, 2017
Merged

test: add async test case#339
popomore merged 1 commit into
masterfrom
add-async-test

Conversation

@dead-horse

@dead-horse dead-horse commented Feb 9, 2017 •
edited by fengmk2
Loading

Copy link
Copy Markdown
Member
Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Description of change

增加 async 相关的测试用例，需要等 egg-core 发布新版和 node 7.6 发布。

@dead-horse

dead-horse commented Feb 9, 2017

Copy link
Copy Markdown
Member Author

从代码到测试都可以用 aa

@dead-horse dead-horse mentioned this pull request Feb 9, 2017
Closed
9 tasks
dead-horse
dead-horse commented Feb 9, 2017
View reviewed changes
Comment thread .eslintrc
"extends": "eslint-config-egg"
"extends": "eslint-config-egg",
"parserOptions": {
"ecmaVersion": 2017

@dead-horse dead-horse Feb 9, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

这个是不是可以放到 eslint-config-egg 里面去了？

@popomore popomore Feb 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

不知道这是不是最低支持版本。

@popomore popomore Feb 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

放到 test/fixtures/apps/async-app 这个下面？

@atian25 atian25 Feb 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@geekdada 看看?

@dead-horse dead-horse Feb 9, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

在 test/async/_async.js 里面也有用，感觉还是把它放到 eslint-config-egg 里面吧，不然别人用 node 7 + async 就比较麻烦了。

@dead-horse dead-horse Feb 9, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@geekdada 会和现在的配置冲突么？如果不冲突的话是不是可以合并成一个。

@geekdada geekdada Feb 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@dead-horse 现行代码的运行环境非 2017，如果默认使用 2017 就没有意义了吧。

@popomore popomore Feb 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

那看起来是可以的，以后很多会用 async function 的

@dead-horse dead-horse Feb 9, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@geekdada 但是会有人基于 node 7.x 开发，要用 async function

@geekdada geekdada Feb 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@dead-horse 我在那边新建一个 pr 大家看一下

dead-horse
dead-horse commented Feb 9, 2017
View reviewed changes
Comment thread test/async/index.test.js

const nodeVersion = Number(process.version.match(/^v(\d+\.\d)+\./)[1]);
// only node >= 7.6 support async function without flags
if (nodeVersion >= 7.6) {

@dead-horse dead-horse Feb 9, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

这样 require 进来就没问题了

@popomore

popomore commented Feb 9, 2017

Copy link
Copy Markdown
Member

这个挂了？

https://nodesecurity.io/orgs/eggjs/projects/0e9b9ae1-72b8-4369-b169-6bd9aca1191f/339#history

@popomore

popomore commented Feb 9, 2017

Copy link
Copy Markdown
Member

schedule 也加个？

@codecov

codecov Bot commented Feb 9, 2017

Copy link
Copy Markdown

Codecov Report

Merging #339 into master will not change coverage.

@@          Coverage Diff          @@
##           master   #339   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          28     28           
  Lines         635    635           
=====================================
  Hits          635    635

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 70eb04f...b72c52c. Read the comment docs.

@codecov

codecov Bot commented Feb 9, 2017 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #339 into master will not change coverage.

@@          Coverage Diff          @@
##           master   #339   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          28     28           
  Lines         635    635           
=====================================
  Hits          635    635

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 70eb04f...fb7247d. Read the comment docs.

@dead-horse

dead-horse commented Feb 9, 2017

Copy link
Copy Markdown
Member Author

schedule 加了

@popomore
popomore merged commit e3532e4 into master Feb 9, 2017
@popomore
popomore deleted the add-async-test branch February 9, 2017 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@atian25 atian25 atian25 left review comments
@geekdada geekdada geekdada left review comments
@fengmk2 fengmk2 fengmk2 approved these changes
@popomore popomore popomore approved these changes

Assignees

No one assigned

Labels

type: test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@dead-horse @popomore @fengmk2 @atian25 @geekdada