The Wayback Machine - https://web.archive.org/web/20200911205148/https://github.com/eggjs/egg/pull/339
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

test: add async test case #339

Merged
merged 1 commit into from Feb 9, 2017
Merged

test: add async test case #339

merged 1 commit into from Feb 9, 2017

Conversation

@dead-horse
Copy link
Member

dead-horse commented Feb 9, 2017

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 requested review from fengmk2 and popomore Feb 9, 2017
@dead-horse
Copy link
Member Author

dead-horse commented Feb 9, 2017

从代码到测试都可以用 aa

@dead-horse dead-horse mentioned this pull request Feb 9, 2017
9 of 9 tasks complete
@dead-horse dead-horse force-pushed the add-async-test branch from 9a88474 to b275040 Feb 9, 2017
"extends": "eslint-config-egg"
"extends": "eslint-config-egg",
"parserOptions": {
"ecmaVersion": 2017

This comment has been minimized.

@dead-horse

dead-horse Feb 9, 2017

Author Member

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

This comment has been minimized.

@popomore

popomore Feb 9, 2017

Member

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

This comment has been minimized.

@popomore

popomore Feb 9, 2017

Member

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

This comment has been minimized.

This comment has been minimized.

@dead-horse

dead-horse Feb 9, 2017

Author Member

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

This comment has been minimized.

@dead-horse

dead-horse Feb 9, 2017

Author Member

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

This comment has been minimized.

@geekdada

geekdada Feb 9, 2017

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

This comment has been minimized.

@popomore

popomore Feb 9, 2017

Member

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

This comment has been minimized.

@dead-horse

dead-horse Feb 9, 2017

Author Member

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

This comment has been minimized.

@geekdada

geekdada Feb 9, 2017

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

@dead-horse dead-horse force-pushed the add-async-test branch from b275040 to b72c52c Feb 9, 2017

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

This comment has been minimized.

@dead-horse

dead-horse Feb 9, 2017

Author Member

这样 require 进来就没问题了

@popomore
Copy link
Member

popomore commented Feb 9, 2017

schedule 也加个?

@codecov
Copy link

codecov bot commented Feb 9, 2017

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
Copy link

codecov bot commented Feb 9, 2017

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.

@fengmk2
fengmk2 approved these changes Feb 9, 2017
@dead-horse dead-horse force-pushed the add-async-test branch from b72c52c to 0a91397 Feb 9, 2017
@dead-horse
Copy link
Member Author

dead-horse commented Feb 9, 2017

schedule 加了

@dead-horse dead-horse force-pushed the add-async-test branch from 0a91397 to fb7247d Feb 9, 2017
@popomore popomore merged commit e3532e4 into master Feb 9, 2017
8 checks passed
8 checks passed
Node Security No known vulnerabilities found
Details
codecov/patch Coverage not affected when comparing 70eb04f...fb7247d
Details
codecov/project 100% remains the same compared to 70eb04f
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
security/snyk No new vulnerabilities
Details
@popomore popomore deleted the add-async-test branch Feb 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.