Skip to content

Add promise support for sessionStore functions#635

Closed
brandonmanke wants to merge 12 commits into
expressjs:masterfrom
brandonmanke:session-store-promise
Closed

Add promise support for sessionStore functions#635
brandonmanke wants to merge 12 commits into
expressjs:masterfrom
brandonmanke:session-store-promise

Conversation

@brandonmanke

@brandonmanke brandonmanke commented Feb 24, 2019

Copy link
Copy Markdown

Potential solution to #607.

I can add some more tests to improve coverage if necessary.

@dougwilson
dougwilson force-pushed the session-store-promise branch from 63f0b88 to c687d5a Compare February 24, 2019 16:38
@brandonmanke

Copy link
Copy Markdown
Author

@dougwilson I can add the should error without callback test for .reload() if that is ok? Also how would you recommend explicitly testing for errors with and without callbacks? Since the tests themselves will be almost identical.

@dougwilson

Copy link
Copy Markdown
Contributor

Hey, so I added a couple more conditions on Monday to further bump the coverage to help out 👍Feel free to add some new ones as well; I just pushed up without the rebase on Monday just in case you were working on some as well.

I took a look at the coveralls config and it's set only to care about line coverage. So I wouldn't really worry about the reject branches to get it landed. If you want to add them anyway as an exercise, I would just copy-paste the tests at this time.

@brandonmanke

Copy link
Copy Markdown
Author

How would you recommend testing https://coveralls.io/builds/21919265/source?filename=index.js#L385?

@bjohansebas

Copy link
Copy Markdown
Member

closes in favor of #737

@bjohansebas bjohansebas closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants