Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Request] Add unit tests #25

Closed
brandl-muc opened this issue Aug 6, 2018 · 9 comments
Closed

[Request] Add unit tests #25

brandl-muc opened this issue Aug 6, 2018 · 9 comments

Comments

@brandl-muc
Copy link
Contributor

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
Copy link
Collaborator

da2x commented Aug 6, 2018

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
Copy link
Contributor Author

brandl-muc commented Aug 6, 2018 via email

@da2x
Copy link
Collaborator

da2x commented Aug 6, 2018

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
Copy link
Contributor Author

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
Copy link
Collaborator

da2x commented Aug 16, 2018

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
Copy link
Contributor Author

brandl-muc commented Dec 23, 2018

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
Copy link
Contributor Author

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
Copy link
Collaborator

da2x commented Jan 2, 2019

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants