Create UI tests with `roku-test-automation`

arturocuya commented 9 months ago

Feature Description

Use the roku-test-automation library to define UI tests for the Jellyfin Roku client. These tests would simulate real user key presses and do assertions over the content's of the Node tree (i.e: what the user can see).

Additional context

Goals

Setup RTA and the dependencies necessary for UI testing.
Create a test suite of simple UI tests, using the stable demo server https://demo.jellyfin.org/stable as a source of information.

Pre-requisite PRs:

https://github.com/jellyfin/jellyfin-roku/pull/1535. This enables the usage of import statements in .js files, which will make test development with RTA easier.
https://github.com/jellyfin/jellyfin-roku/pull/1537. RTA requires a reference to the device's host and password. Currently Roku Jellyfin developers set that on their VSCode user configuration. This change enables credentials configuration from bsconfig.json, which is equally easy to deal with as the VSCode config, but it's also a place that RTA can access to get the credentials.

arturocuya commented 9 months ago

Maintainers please LMK if you agree with the goals.

Also, When launching the app programatically, I encountered this crash:

Suspending threads...
Thread selected:  1*   pkg:/components/JFOverhang.brs(22)         if not m.hideClock

Current Function:
014:      ' save node references
015:      m.title = m.top.findNode("overlayTitle")
016:      m.overlayRightGroup = m.top.findNode("overlayRightGroup")
017:      m.overlayTimeGroup = m.top.findNode("overlayTimeGroup")
018:      m.slideDownAnimation = m.top.findNode("slideDown")
019:      m.slideUpAnimation = m.top.findNode("slideUp")
020:      ' show clock based on user setting
021:      m.hideClock = m.global.session.user.settings["ui.design.hideclock"]
022:*     if not m.hideClock
023:          ' save node references
024:          m.overlayHours = m.top.findNode("overlayHours")
025:          m.overlayMinutes = m.top.findNode("overlayMinutes")
026:          m.overlayMeridian = m.top.findNode("overlayMeridian")
Source Digest(s): 
pkg: dev 1.6.6 e40d407d Jellyfin

Type Mismatch. Operator "not" can't be applied to "Invalid". (runtime error &h18) in pkg:/components/JFOverhang.brs(22)

What the script is doing is running npm run build and creating a .zip from build/staging. It's weird that this doesn't happen when running the app from the Run & Debug panel.

A solution I found is adding this code to components/JFOverhang.bs

    ' show clock based on user setting
    m.hideClock = m.global.session.user.settings["ui.design.hideclock"]
    if m.hideClock = invalid ' <<< new check
        m.hideClock = false
    end if
    if not m.hideClock
        ' ...
    end if

But I'm not sure if false would be the right default value.

1hitsong commented 9 months ago

I think this sounds great. One thing to document and keep in mind is the demo site does reset every hour, so there may be times the tests fail because the user happened to have run them at a moment the site was down.

1hitsong commented 9 months ago

But I'm not sure if false would be the right default value.

This is a user setting, so you can check the settings.json file and see what the default value for the setting is. You could then do a isValid() check on the setting and if not isValid(m.hideClock) set the value to the default value.

cewert commented 9 months ago

This is a user setting, so you can check the settings.json file and see what the default value for the setting is. You could then do a isValid() check on the setting and if not isValid(m.hideClock) set the value to the default value.

I am loading default values from the json settings file on session.Init() so I think his error is showing us a bad path/bug somewhere. He says When launching the app programatically so I'm guessing it has something to do with that.

arturocuya commented 9 months ago

I think this sounds great. One thing to document and keep in mind is the demo site does reset every hour, so there may be times the tests fail because the user happened to have run them at a moment the site was down.

Yes, for the initial tests I'll try to implement general behavior checks that have to happen regardless of the content provided. I.e: Won't test state changes because the initial state can't be guaranteed.

cewert commented 9 months ago

Also, When launching the app programatically, I encountered this crash:

I'd like to help figure out this crash you are having. Can you add a print statement above the crash to print out the current user settings? These should all be set to their default values from settings.json.

print "m.global.session.user.settings = ", m.global.session.user.settings

Also, how exactly are you launching the app?

What the script is doing is running npm run build and creating a .zip from build/staging. It's weird that this doesn't happen when running the app from the Run & Debug panel.

It was either by accident or I thought we didn't need the zip. Feel free to change it if it makes sense and improves your workflow somehow.

arturocuya commented 9 months ago

@cewert I found the issue. RTA lets you deploy with a command like so

await device.deploy({ project: './bsconfig-rta.json', rootDir: './build/staging' });

By including project, I assumed it would use the files configuration from there. But RTA was overwriting it with a default list that does not include the settings folder. Hence the crash.

RIGHTSCRIPT: ERROR: ReadAsciiFile: file open for read failed: pkg:/source/utils/config.brs(6)
BRIGHTSCRIPT: ERROR: ParseJSON: Data is empty: pkg:/source/utils/config.brs(6)
m.global.session.user.settings =                <Component: roAssociativeArray> =
{
}

Adding the files array manually with the correct value fixes the problem.

    await device.deploy({
        project: './bsconfig-rta.json', rootDir: './build/staging', files: [
            "manifest",
            "source/**/*.*",
            "components/**/*.*",
            "images/**/*.*",
            "resources/**/*.*",
            "locale/**/*.*",
            "settings/*.*"
        ]
    });

m.global.session.user.settings =                <Component: roAssociativeArray> =
{
    global.rememberme: false
    itemgrid.gridTitles: "showonhover"
    itemgrid.movieDefaultView: "movies"
    ...

I will submit an issue to roku-test-automation so that they can fix it on their side too.

arturocuya commented 9 months ago

Yes, for the initial tests I'll try to implement general behavior checks that have to happen regardless of the content provided. I.e: Won't test state changes because the initial state can't be guaranteed.

@cewert @1hitsong any ideas on critical paths untied to server state that would be good to cover for?

cewert commented 9 months ago

any ideas on critical paths untied to server state that would be good to cover for?

You mean what can we test without connecting to and depending on a server? The only part of our app that functions without a server connection would be the "server select screen" that is displayed on first boot aka SetServerScreen.bs which is created by CreateServerGroup() and called by LoginFlow() on line 46 of main.bs. Possible things to test would be the auto server discovery feature, inputting a server URL, selecting the submit button, and making sure back exits the app. Not sure if that's what you're looking for but that's all I can think of (you can also delete a saved server using * but you need to have connected to a server for it to save)

arturocuya commented 9 months ago

No, what I mean is: Are there any behaviors that can be replicated regardless of the current contents of the server? For example, if it's guaranteed that the server will always return 3 categories in "My media", and that more than 1 video is sent for the "Movies" category, we can check that the values for the title, metadata and description change when you select another element from the movies list. For one video (any video) we can check that clicking on it and then on "play" starts video playback.

Those kinds of things, where specific values don't matter and instead it's important that the server sends a consistent amount of data.

cewert commented 9 months ago

Oh gotcha. That's a much harder question I need to think about this 😄 There's no guarantee that the server has any media at all even after we connect to it. and if it does have media it could be of any type.

I think we would have to make some assumptions about server state to get to what you're wanting to test. For your example, we would have to assume there is at least one library created and there is at least one library with content type of "movies".

Possible content types when making a library in jellyfin: jellyfin-content-types

jellyfin / jellyfin-roku