Bunchhieng / hnreader

Open programming news with your favorite browser using command line
https://bunchhieng.github.io/hnreader/
MIT License
7 stars 10 forks source link

Refactor getBrowserNameByOS #19

Closed Xercoy closed 5 years ago

Xercoy commented 5 years ago

getBrowserNameByOS doesn't have unit tests, so I decided to add one.

Right now, it's basically a subtest that runs based on the value of runtime.GOOS since the way getBrowserNameByOS is written is only meant to handle cases where we're dealing with a darwin based OS. Not only does the unit test do that, but it tests every possible denormalized string value.

I was thinking that since the function only handles cases for a darwin OS, we can ensure that any other value for runtime.GOOS will result in a return value of a null string. While it does adhere to the current behavior, I feel like it could be overkill.

The code for it also seemed like it could have been simplified a bit for a few reasons:

To ensure that my changes to the function work as intended, I created the test, ran the entire test suite and ensured it passed, modified getBrowserNameByOS, then ran tests a final time to ensure its behavior remained unchanged.

Bunchhieng commented 5 years ago

Would you be able to fix the merge conflict? @Xercoy

Xercoy commented 5 years ago

@Bunchhieng I'll work on it and ping you when it's done. Apologies for the delay.

Xercoy commented 5 years ago

closing in favor of #50