Moving mockfirebase forward
jamestalmage opened this issue · 19 comments
@katowulf @jwngr
With Ben taking a step back, where does mockfirebase stand? We have now lagged behind Firebase by at least a couple releases. Should we deprecate and just point people towards firebase-server? It would mean giving up on the nice synchronous flush features in mockfirebase, but writing async tests is a small price to pay if it means your test framework stays up to date with reality. I know firebase-server actually depends on mockfirebase, but looking at the codebase, it really is using a very small subset of the API.
@bendrucker
I have poked around the v1 branch, and what I see looks good. Did you have any sort of checklist / outline for what still needs to be completed?
I don't have an outline of what's left to do. I can provide a little bit of support here and there if needed.
So,
I just ran a quick test against firebase-server and it already supports orderBy queries and the atomic deep writes.
https://gist.github.com/jamestalmage/21d34205529136acc6a8?ts=2
That is impressive... and pretty demotivating when it comes to continued mockfirebase development.
cc @urish
Care to weigh in here? Anything missing from firebase-server where mockfirebase is the blocker? Anything more we can do on our end to help support your efforts?
@jamestalmage actually, queries don't work reliably, as they are not implemented by the server. as far as I can say, the client itself orders the data when you query by orderBy, but then, if you used it on conjunction with startAt / endAt / equalTo / limitToLast, I believe that it wouldn't work as expected as there is no server-side support for that yet.
So, as far as I can tell, queries are definitely desired from the firebase-server side.
@urish check the gist I posted above.
excerpt:
var query = statesRef.orderByValue().startAt('Delaware').limitToFirst(10);
that seemed to work, even with ref.update using the deep-path syntax.
I perused all the old issues in your repo and didn't see one regarding this.
Obviously, the server is not being "smart" about what it sends down to the client (it is sending everything, despite the fact it's a limited query), but I can't see why that is a problem for tests.
@jamestalmage I have done the tests a few months ago, so I can't recall the exact scenario that makes it fail. I will try to reproduce and update you...
@jamestalmage after experimenting with it for a while, I could only cause priorities to fail. For example, the following piece of code:
var ref = new Firebase('ws://test.firebaseio.com:5000');
var statesRef = ref.child('states');
var query = statesRef.startAt(1);
ref.child('states/AL').setWithPriority('Alabama', 200);
ref.child('states/FL').setPriority(100);
query.on('value', function(snap) {
console.log('Current value: ', snap.val());
});
produces:
Current value: { FL: 'Florida', AL: 'Alabama' }
Current value: { FL: 'Florida' }
Current value: null
While it should obviously return both FL and AL as the final value. I haven't implemented Priorities on the firebase-server side, so perhaps it has nothing to do with mock-firebase anyway...
That output looks right, but in reverse order.
This test works fine: removed see with priority in gist
@urish
I do think it is on the firebase-server end. I will file an issue there. Let's keep the discussion here on the future of mockfirebase.
@jamestalmage great
Thanks for the discussion everyone. And sorry for the dead silence from Kato and I. Speaking frankly, I don't see a lot of time for Kato or I to pick up where Ben left off and bring mockfirebase back up the current version of Firebase. We are both swamped with other work and unfortunately things like mockfirebase have fallen to the wayside. Without someone on our team devoted to keeping this updated, I'm not sure how long it will be after future releases before mockfirebase is updated. This is not an officially supported library (which is why it's not on the firebase GitHub) and we unfortunately don't have the manpower to devote a lot of time to it.
I wish I had better news to share. I'm going to keep pushing users to firebase-server and suggest that if anyone wants to see mockfirebase support a certain feature, that they send us a PR for it.
Well turns out Firebase itself can be turned into a mockfirebase equivalent pretty easily.
So, from what I can gather mockfirebase is no longer being maintained? Are you looking for new maintainers/contributors? @bendrucker @katowulf
Okay, so first of all, let's set expectations here:
This is an experimental library and is not supported by Firebase
This was an early solution for testing with Firebase. As I've grown more experienced with real-time and particularly with integrating it into frameworks like Angular, which provide sophisticated e2e tools, I've realized it's not the best answer here--it probably creates more nuanced behaviors and false positives than it helps correct.
Generally speaking, your unit tests shouldn't be testing third party tools (they should do that, and Firebase does). Mock the methods with a couple good spies. For things like auth and MVC testing, use your e2e tests and go to the source.
See AngularFire's unit tests and e2es for examples. Here's a great example unit test using spies, which I suggest using as a template.
If you feel AngularFire has some value to the community, I'd be happy to accept PRs. But without a perceived need, I'm not willing to devote much time here.
@katowulf I understand the expectations and just needed clarity on whether this library was still being watched. I do see value in mockfirebase so I wouldn't mind helping out maintain the library.
I do agree with you that spies can be used to achieve the same thing as mockfirebase for unit testing, however, spies don't scale. Large scale projects would need to have spies everywhere and if the firebase API changes, trying to update those spies would be a nightmare. In the end you'll find yourself refactoring the spies into a mock object which will ultimately resemble mockfirebase.
Mocks should be dumb, so I do agree with you that there are cases where mockfirebase will not resemble the true functionality of firebase and yield false positives or false negatives. In these situations, integration or e2e testing should be in place to verify the results. Unit testing is not a catch all, but it should be used as the first resort to testing your code.
@soumak77 - check out firebase-server. It's a better solution for integration testing, and since it uses Firebase code itself under the hood, it's easier to keep up to date.
@jamestalmage I agree that firebase-server should be used for integration or end-to-end testing, but it does not fit for unit testing