Skip to content

fix(test): more tests and WebExecuter changes#425

Merged
AVVS merged 11 commits intomakeomatic:masterfrom
pajgo:fix/facebookTestAddons
Sep 19, 2019
Merged

fix(test): more tests and WebExecuter changes#425
AVVS merged 11 commits intomakeomatic:masterfrom
pajgo:fix/facebookTestAddons

Conversation

@pajgo
Copy link
Copy Markdown
Collaborator

@pajgo pajgo commented Sep 10, 2019

No description provided.

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 10, 2019

Codecov Report

Merging #425 into master will increase coverage by 0.06%.
The diff coverage is 95.65%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #425      +/-   ##
=========================================
+ Coverage   94.53%   94.6%   +0.06%     
=========================================
  Files         130     130              
  Lines        2380    2391      +11     
=========================================
+ Hits         2250    2262      +12     
+ Misses        130     129       -1
Impacted Files Coverage Δ
src/auth/oauth/index.js 97.22% <100%> (+2.48%) ⬆️
src/auth/oauth/utils/errors.js 95% <94.44%> (-5%) ⬇️
src/actions/remove.js 95.65% <0%> (-0.27%) ⬇️

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 7ab3be2...c911a6c. Read the comment docs.

Comment thread test/suites/oauth/facebook.js Outdated

});

it.skip('errors from @hapi/bell passed through', async () => {
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.

тут скип?

* WebExecuter new Error on waitFor* timeout with additional data
* Rework oauth test logic - remove unneded actions
* Additional tests
* error serialization
* code cleanup
@pajgo pajgo force-pushed the fix/facebookTestAddons branch from 6371730 to e3b99fd Compare September 15, 2019 13:05
@pajgo pajgo marked this pull request as ready for review September 15, 2019 14:34
Comment thread src/auth/oauth/utils/errors.js Outdated
}

/* Recursive check */
if (error.data instanceof Boom) {
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.

https://github.com/hapijs/boom/blob/master/API.md#isboomerr это нужно использовать нежели instanceof

Comment thread src/auth/oauth/utils/errors.js Outdated
let innerErrorJsonObj;

/* Boom error contains additional data */
if (innerError instanceof Boom) {
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.

console.error('failed to rejectAuth', e);
await page.screenshot({ fullPage: true, path: `./ss/declined-${Date.now()}.png` });
throw e;
throw await this.processPageError(e);
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.

возможно стоит await this.processPageError(e) сделать, а внутри просто кидать ошибку? (там где return ... error)

throw await странная конструкция

@AVVS
Copy link
Copy Markdown
Member

AVVS commented Sep 16, 2019

LGTM, после запрошенных изменений

* remove throw await construction
uri: `${userId}`,
method: 'DELETE',
});
static deleteTestUser(user) {
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.

static async тогда, иначе в одном случае будет промис, в другом просто null (во всех функциях так)

* isBoom fix
* throw await remove
* graph api empty user check
* typo fix
@pajgo pajgo force-pushed the fix/facebookTestAddons branch from e85ad51 to f64c8af Compare September 18, 2019 18:04
* Error serialization tests
* GraphApi check user set
* Split Executer and GraphApi calls in hooks
@AVVS AVVS merged commit a82eb9b into makeomatic:master Sep 19, 2019
@pajgo pajgo deleted the fix/facebookTestAddons branch September 20, 2019 10:51
@AVVS
Copy link
Copy Markdown
Member

AVVS commented Oct 3, 2019

🎉 This PR is included in version 11.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@AVVS AVVS added the released label Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants