firebase / firebase-admin-java

Firebase Admin Java SDK
https://firebase.google.com/docs/admin/setup
Apache License 2.0
525 stars 253 forks source link

[FR] Relax encapsulation for Firebase Messaging exception and Messages payload to enable testability #695

Open cranberrysoft opened 2 years ago

cranberrysoft commented 2 years ago

Currently, unit testing for integration with Firebase Messaging is very challenging. The encapsulation imposed by the final and visibility of the method makes it untestable. On top of this, there is no emulator for Firebase messaging which could help with testing integration with a more production-like environment.

1) I want to emulate exceptions on FCM Messaging like UNREGISTERED, INVALID_ARGUMENT when calling com.google.firebase.messaging.FirebaseMessaging#send(com.google.firebase.messaging.Message) to test my error handling implementation e.g deleting invalid token

2) I want to unit test if a message with a given payalod was sent to FCM Currently FCM payload represented with com.google.firebase.messaging.Message is not accessible to third parties via getters.

In this case, developers used @VisibleForTesting for their purposes for testing while they totally forgot about developers who use their library and can not access the payload of the message. On top of this, there is no provided reasonable toString to make any assertions based on this.

Describe the solution you'd like Exceptions testing

Messages testing

Describe alternatives you've considered

Additional context Too aggressive encapsulation in my opinion is harmful. Developers should balance encapsulation and testability, taking into account the users of their libraries not only their own goals to protect their code with all means available in Java which are not perfect

google-oss-bot commented 2 years ago

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

rodrigo-om commented 6 months ago

+1 on this

t-beckmann commented 1 month ago

+1, annoying

leolaurindo commented 1 month ago

+1