Skip to content
This repository was archived by the owner on Mar 24, 2026. It is now read-only.

Hapi fix#2424

Merged
NateBrady23 merged 2 commits intoTechEmpower:masterfrom
NateBrady23:js-hapi
Dec 28, 2016
Merged

Hapi fix#2424
NateBrady23 merged 2 commits intoTechEmpower:masterfrom
NateBrady23:js-hapi

Conversation

@NateBrady23
Copy link
Copy Markdown
Member

The default test for hapi was also trying to spin up redis. We currently do not have a working redis setup.

@LadyMozzarella
Copy link
Copy Markdown
Contributor

Could we just delete the implementation instead of commenting it out?

@NateBrady23
Copy link
Copy Markdown
Member Author

@LadyMozzarella We may be adding redis back in, so for tests that are failing because redis was removed I was just planning on keeping the author's original implementation in code. It may be helpful for people who are looking at the current implementation, wondering what the intent was. Since I don't know if they would have done things differently had they known they couldn't use redis, this might be the best way for now.

@NateBrady23
Copy link
Copy Markdown
Member Author

Nonetheless, this one passed locally but isn't passing travis so I will have to revisit in a moment.

@LadyMozzarella
Copy link
Copy Markdown
Contributor

In that case, can you add a comment with that information while you're in there?

@NateBrady23
Copy link
Copy Markdown
Member Author

Will do.

@NateBrady23
Copy link
Copy Markdown
Member Author

Because of the way the author wrote this test, (and possibly the way hapi works), a working db connection is expected on startup. Combined the default test with the mongodb tests to make it the default.

@NateBrady23
Copy link
Copy Markdown
Member Author

All green! merging

@NateBrady23 NateBrady23 merged commit 39abd9a into TechEmpower:master Dec 28, 2016
@NateBrady23 NateBrady23 deleted the js-hapi branch December 28, 2016 21:16
zloster pushed a commit to zloster/FrameworkBenchmarks that referenced this pull request Mar 21, 2017
* remove redis implementation

* combine default test
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants