GNOME Bugzilla – Bug 741784
lua-factory: testing
Last modified: 2015-08-25 16:17:02 UTC
Important tests related to lua-factory are: - tests for lua sources; - tests for lua-library; I'm starting with the simplest part, enabling tests for the a lua source and a tests for metrolyrics plugin; I'm not sure yet what is the best approach for testing lua-library, atm I would like to: 1-) Create a lua module from lua-library 2-) Lua scripts using the module above to tests de functions 3-) Evaluate the results of lua scripts the same way that we do in other sources (g_test_run) Issues: - Some functions use lua-factory structure (OperationSpec *) to deal with async operations like grl.fetch - (probably more) After having proper tests, documentation would be the next step :-)
Created attachment 293091 [details] [review] tests: enable local tests with lua-factory
Created attachment 293092 [details] [review] tests: grl-metrolyrics (lua source)
> I'm not sure yet what is the best approach for testing lua-library, atm I would > like to: > 1-) Create a lua module from lua-library > 2-) Lua scripts using the module above to tests de functions > 3-) Evaluate the results of lua scripts the same way that we do in other > sources (g_test_run) > > Issues: > - Some functions use lua-factory structure (OperationSpec *) to deal with async > operations > like grl.fetch > - (probably more) Actually, having a fake-lua-source for test looks much better and simpler...
Review of attachment 293091 [details] [review]: Sure.
Review of attachment 293092 [details] [review]: The rest looks good enough though. ::: tests/lua-factory/sources/test_lua_metrolyrics.c @@ +26,3 @@ +#define METROLYRICS_OPS GRL_OP_RESOLVE + +#define LYRICS_RING_OF_FIRE \ The lyrics definitely need to be in a separate file, one for each song, that we'd either load from disk, or use GResources to embed.
Comment on attachment 293091 [details] [review] tests: enable local tests with lua-factory Attachment 293091 [details] pushed as cc55425 - tests: enable local tests with lua-factory
Created attachment 298564 [details] [review] tests: grl-metrolyrics (lua source) * Split lyrics in several files and use GResource;
Created attachment 298565 [details] [review] lua-factory: fix change in metrolyrics (website) HTML part of the lyrics changed. I've changed the get_lyrics to clean up HTML that cames with the lyrics instead of matching it.
Created attachment 298566 [details] [review] lua-factory: Fix makefile rule to euronews resource The rule to make euronews GResource uses guardianvideos's resource description as dependency.
PS: I still want to do lua-library tests with a 'fake' lua source as per #c3
Review of attachment 298564 [details] [review]: Apart from that, looks good. ::: tests/lua-factory/sources/test_lua_metrolyrics.c @@ +96,3 @@ + g_assert_no_error (error); + + data = g_bytes_get_data (bytes, &size); GFile *file = g_file_new_for_uri ("resources:///org/gnome/grilo/plugins/test/metrolyrics/data/foo.txt"); g_file_load_contents(file, ...); Simpler.
Review of attachment 298565 [details] [review]: > lua-factory: fix change in metrolyrics (website) Update for change in metrolyrics website > HTML part of the lyrics changed. I've changed the get_lyrics to clean up > HTML that cames with the lyrics instead of matching it. "came" (there's no 's' at the third-person singular in preterit)
Review of attachment 298566 [details] [review]: Yes.
pushed as a8a419d112e234d3fe8c7e843b0ab0f64c191cbf Keeping the bug open for the lua-library tests
Review of attachment 298565 [details] [review]: This was commited as 31f4af07be38e2a66112adda35b99360507d2397
Review of attachment 298564 [details] [review]: This was commited as a8a419d112e234d3fe8c7e843b0ab0f64c191cbf