search

electricitymaps / bloom-contrib

Making carbon footprint data available to everyone.
https://www.bloomclimate.com
MIT License
434 stars 104 forks source link

Energinet integration tests #462

Closed pierresegonne closed 3 years ago

pierresegonne commented 3 years ago

Could potentially add test for:

I would also like to point out the use of global.fetch, that I don't find optimal at all.

Also, what do you guys think about having some shared methods for testing integrations?

madsnedergaard commented 3 years ago

Could potentially add test for:

  • [ ] Invalid token = connect failure

I think it's okay without this - I'd say going for 100% test coverage is rarely worth the effort, especially at the stage we're at right now :)

I would also like to point out the use of global.fetch, that I don't find optimal at all.

Ideally we should use fetch everywhere and not superagent (which is also much more complicated to mock) :)

I actually just checked, and fetch is available in react-native (at least now), so not sure why we used superagent initially 🤔 Any idea @corradio?

Also, what do you guys think about having some shared methods for testing integrations?

Might make sense, but let's wait with optimizing before we actually need it in multiple tests. And if you're thinking of the superagent mocking things, then I'd much rather kill that mess instead 😅

corradio commented 3 years ago

I actually just checked, and fetch is available in react-native (at least now), so not sure why we used superagent initially 🤔 Any idea @corradio?

Also, what do you guys think about having some shared methods for testing integrations?

Might make sense, but let's wait with optimizing before we actually need it in multiple tests. And if you're thinking of the superagent mocking things, then I'd much rather kill that mess instead 😅

superagent is used in places where cookies are required if I remember correctly. Normally fetch should be preferred. Are you able to put me as reviewer on the PRs where you migrate from fetch to superagent if/when you do? In theory I see no hurdle for those without cookies.

madsnedergaard commented 3 years ago

superagent is used in places where cookies are required if I remember correctly. Normally fetch should be preferred. Are you able to put me as reviewer on the PRs where you migrate from fetch to superagent if/when you do? In theory I see no hurdle for those without cookies.

Cool, thanks! I'd probably suggest we wait with migrating for now, just wanted to get some context and see if it was even possible :)