[Request] Add unit tests

stuartlangridge / gnome-shell-clock-override

Override the Gnome Shell clock with a new time format or text of your choice. Works with new versions of Shell such as 3.18

MIT License

89 stars 33 forks source link

[Request] Add unit tests #25

Closed brandl-muc closed 4 years ago

brandl-muc commented 5 years ago

I'm not at all js savvy nor would I know how to achieve this, but I think it might by a good idea to add tests to the project to avoid that the plugin breaks like with #13 . The bare minimum seems to be to test overrider() in extension.js

Any ideas or comments on this?

da2x commented 5 years ago

I looked into this but couldn’t decide on a test framework. (Yeah, really.) Tests can be run with the gjs command directly, which means GLib and the other imports will work in tests. The enable() and disable() functions aren’t testable, however.

A pull request would be very welcome.

brandl-muc commented 5 years ago

As I said, may js knowledge is minimal. But I'll give it a try, hopefully soon.

Daniel Aleksandersen notifications@github.com schrieb am Mo., 6. Aug. 2018, 22:18:

I looked into this but couldn’t decide on a test framework. (Yeah, really.) Tests can be run with the gjs command directly, which means GLib and the other imports will work in tests. The enable() and disable() functions aren’t testable, however.

A pull request would be very welcome.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/stuartlangridge/gnome-shell-clock-override/issues/25#issuecomment-410840116, or mute the thread https://github.com/notifications/unsubscribe-auth/AXJQYc6WX0cruYq2zaB82ZZQOXsoiz4Aks5uOKSogaJpZM4Vw_GL .

da2x commented 5 years ago

Refactor plan to make things more easily testable:

move format processing out of overrider() to separate formatting function. Format function should return fully formatted strings.
create GLib.DateTime in overrider() and supply it to the new formatting function. Means we can supply a mock DateTime and do direct string comparisons of what comes out of the formatter function.
probably separate out each individual formatter extension in their own functions also.

brandl-muc commented 5 years ago

That sounds like a very good plan.

Meanwhile I found this: Is there any way to write unit tests for GNOME-Shell extensions This gives rise to some questions:

I'd like to add travis support, but for our repo you would need to activate that of course. So if you're not going to do this I can as well not add support. Opinion?
How do you deploy your extension?
In consequence, would it be possible to change the directory structure to having a src and a test directory just as in the linked power-alt-tab?

da2x commented 5 years ago

Power Alt-Tab’s approach looks promising.

I'd like to add travis support, but for our repo you would need to activate that of course. So if you're not going to do this I can as well not add support. Opinion?

I don’t have permissions to do this, but I’m sure @stuartlangridge can assist in enabling it. However, the tests should be runnable locally too (make test) so we should get that working first.

How do you deploy your extension?

make dist (see the Makefile) produces the extension, and then I manually upload it.

In consequence, would it be possible to change the directory structure to having a src and a test directory just as in the linked power-alt-tab?

Why would the sources need to move? It messes with their history. Just stick tests in a subdirectory on it’s own. It’s common to keep them in tests.

brandl-muc commented 5 years ago

All right, quite busy these last months (a wedding and a move) but I finally found some time and gave it a first try. See PR #30 ...

brandl-muc commented 5 years ago

Ah, one thing, any idea why the %r format string does not work on my system?

gjs> const GLib = imports.gi.GLib;
gjs> var now = GLib.DateTime.new_now_local();
gjs> print(now.format("%R"));
19:18
gjs> print(now.format("%r"));
null

Consequently using %r in the extension basically turns it off.

da2x commented 5 years ago

Thank you for your work so far, @brandl-muc!

Ah, one thing, any idea why the %r format string does not work on my system?

This may be locale dependant. Try switching to the en-US locale. It may have been set to an empty string in your locale, but that should also return as an empty string and not null.

brandl-muc commented 5 years ago

Created PR #33 to include my tests, feel free to critique.

However since this needs jasmine-gjs just as power-alt-tab does, I didn't add anything to the makefile yet. It does not seem sensible to check out and install jasmine-gjs by issuing a makefile command. Suggestions are very welcome, otherwise we can also leave it like is. This works well for local execution and Travis integration should be easy. (hopefully)

Next step would be breaking the format function down to smaller functions as you suggested, afterwards I would propose to close the issue.