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.

Sign up for GitHub

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

[FLAG] Adds keepAppOpen flag #212

Merged
merged 3 commits into from
Apr 19, 2023
+22 −2
Merged

[FLAG] Adds keepAppOpen flag #212

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3b3beec
Adds keepAppOpen flag
twig2let Jan 30, 2023
3b9c65b
Merge branch 'master' into ci-support
twig2let Apr 19, 2023
6728b15
test fix - ensures keepAppOpen is true by default
twig2let Apr 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bsc-plugin/src/lib/rooibos/RooibosConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export interface RooibosConfig {
tags?: string[];
catchCrashes?: boolean;
sendHomeOnFinish?: boolean;
keepAppOpen?: boolean;
/**
* The path to the folder where the rooibos framework roku files reside.
* @default `dist/lib/framework`
Expand Down
1 change: 1 addition & 0 deletions bsc-plugin/src/lib/rooibos/RooibosSession.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export class RooibosSession {
"printLcov": ${this.config.printLcov ? 'true' : 'false'}
"port": "${this.config.port || 'invalid'}"
"catchCrashes": ${this.config.catchCrashes ? 'true' : 'false'}
"keepAppOpen": ${this.config.keepAppOpen ? 'true' : 'false'}
}`
)
);
Expand Down
4 changes: 3 additions & 1 deletion bsc-plugin/src/plugin.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1295,6 +1295,7 @@ describe('RooibosPlugin', () => {
"printLcov": false
"port": "invalid"
"catchCrashes": false
"keepAppOpen": true
}
end function
instance.getTestSuiteClassWithName = function(name)
Expand Down Expand Up @@ -1357,7 +1358,8 @@ describe('RooibosPlugin', () => {
'catchCrashes': true,
'lineWidth': 70,
'failFast': false,
'sendHomeOnFinish': false
'sendHomeOnFinish': false,
'keepAppOpen': false
},
'maestro': {
'nodeFileDelay': 0,
Expand Down
3 changes: 3 additions & 0 deletions bsc-plugin/src/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ export class RooibosPlugin implements CompilerPlugin {
if (config.isRecordingCodeCoverage === undefined) {
config.isRecordingCodeCoverage = true;
}
if (config.keepAppOpen === undefined) {
config.keepAppOpen = true;
}
//ignore roku modules by default
if (config.includeFilters === undefined) {
config.includeFilters = [
Expand Down
1 change: 1 addition & 0 deletions docs/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ The following options are supported:
- lineWidth?: number - width of test output lines in columns
- catchCrashes? : boolean - if true, then any crashes will report CRASH statement, and note halt test execuction - very useful for running a whole suite
- sendHomeOnFinish? : boolean - if true, then the app will exit upon finish. The default is true. Useful to set to false for local test suites.
- keepAppOpen? : boolean - when true, the app will remain open upon test completion. The default is true. Set false to return execution to Main.
- testsFilePattern?: string - the pattern to use to find tests, this is a glob, the default is "**/*.spec.bs"
- tags?: string[] - the tags listed here control what is run - you can use !tagname to indicated any tests/suites that are skipped, all other tags are ANDed. This is very useful for having a bsconfig to run, say tests including long, and slow integration tests, or just running a certain subset of your suite.

Expand Down
5 changes: 5 additions & 0 deletions framework/src/source/Rooibos.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ namespace rooibos
? "scene is ready; running tests now"
runner = new rooibos.TestRunner(scene, m)
runner.Run()

if runner.config.keepAppOpen = false
? "keepAppOpen is false; exiting Rooibos"
return
end if
end if

while true
Expand Down
6 changes: 6 additions & 0 deletions framework/src/source/TestRunner.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,12 @@ namespace rooibos

m.testReporter.reportResults(m.stats)

rooibosResult = {
stats: m.stats
testSuites: m.testSuites
}
m.nodeContext.global.testsScene.rooibosTestResult = rooibosResult
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you pick storing results in the scene? Should we use GetGlobalAA() instead? Or would there be no difference?

Suggested change
m.nodeContext.global.testsScene.rooibosTestResult = rooibosResult
GetGlobalAA().rooibosTestResult = rooibosResult

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to support a ci socket connection; but it's against the spirit of the project to facilitate people using it for their own ci, without providing examples of how others can benefit. I'll not merge any such solution that facilitates peoples private ci's without some help for other users. I at least need some documentation with a sample snippet of code. I'd much rather someone provides a sample js. I think that's a reasonable ask.

Copy link
Collaborator

@georgejecook georgejecook Apr 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't recall. it was years ago, and I was solving really really hard problems while learning roku and trying to write this test framework. could have been something back in the os then that was a problem, could have been I was overwhelmed, or just didn't know enough yet. Happy to have this change if you're sure it doesnt break anything.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already shared examples of code on the brs side of things. The gulp side for my specific project (which I've been cleared to share) looks something like this:

var net = require('net');
var fs = require('fs');

module.exports = function (gulp, plugins) {
    return cb => {
        const ROKU_DEV_TARGET = process.env.ROKU_DEV_TARGET;
        const RETRY_DELAY = 500;
        const socket = net.Socket();

        let remainingConnectionAttempts = 5;

        getResults()
            .then(parseResults)
            .then(saveResults)
            .then(results => {
                if (results.success) {
                    console.log('Tests passed! \n')
                } else {
                    console.log(`\n ${results.failedTestCount} ${results.failedTestCount > 1 ? 'Tests' : 'Test'} failed!  \n`);
                }
                cb();
            })
            .catch(err => {
                console.log(`\n Error: ${err} \n`);
                cb(err);
            });

        function getResults() {
            return new Promise((fulfil, reject) => {
                let dataStr = '';

                socket.setEncoding('utf8');
                socket.setKeepAlive(false);
                socket.setTimeout(180000, () => {
                    socket.end();
                });

                connect();

                socket.on('connect', () => {
                    console.log(`\n Connected to ${ROKU_DEV_TARGET} \n`);
                });

                socket.on('data', data => {
                    dataStr = dataStr + data;
                });

                socket.on('error', err => {
                    errorMessage = `Unable to get response from box ${ROKU_DEV_TARGET}: ${err}`;
                    remainingConnectionAttempts--;

                    if (remainingConnectionAttempts < 0) {
                        reject(errorMessage);
                        return;
                    }

                    console.log(errorMessage);
                    console.log(`Retrying connection in ${RETRY_DELAY}ms. Remaining attempts: ${remainingConnectionAttempts}`)
                    setTimeout(connect, RETRY_DELAY);
                });

                socket.on('end', () => {
                    console.log('Closing Socket!');
                    fulfil(dataStr);
                    console.log(`\n Disconnected from ${ROKU_DEV_TARGET} \n `);
                });
            });
        }

        function parseResults(resultStream) {
            return new Promise((fulfil, reject) => {
                rooibosResult = JSON.parse(resultStream);

                let xml = `
                <testsuites>
                    <testsuite name="Rooibos" tests="${rooibosResult.totalTestCount}" failures="${rooibosResult.failedTestCount}">\n`;

                rooibosResult.tests.forEach((test, index, tests) => {
                    if (!test.isFail) {
                        xml += `
                        <testcase name="Passed Test ${index}" classname="${test.name}"/>`
                    } else {
                        xml += `
                        <testcase name="${test.name}" classname="${test.filePath}-FAIL">
                            <failure message="${test.message}"/>
                        </testcase>`
                    }
                    xml += '\n';
                });

                xml += `
                    </testsuite>
                </testsuites>
                `

                fulfil({
                    xml: xml,
                    success: rooibosResult.success,
                    empty: rooibosResult.totalTestCount == 0,
                    failedTestCount: rooibosResult.failedTestCount
                });
            });
        }

        function saveResults(results) {
            return new Promise((fulfil, reject) => {
                if (results.empty == false) {
                    const testResultsLoc = process.env.TEST_RESULTS_LOC || './source/tests/results/';
                    if (!fs.existsSync(testResultsLoc)) {
                        fs.mkdirSync(testResultsLoc);
                    }
                    fs.writeFile(testResultsLoc + 'test-results.xml', results.xml, err => {
                        if (err) {
                            reject(err);
                        }
                        console.log('\n Results saved to ' + testResultsLoc + 'test-results.xml \n ');
                    });
                }
                fulfil(results);
            });
        }

        function connect() {
            socket.connect(global.args.rooibosPort, ROKU_DEV_TARGET);
        }
    };
};

It just connects to the brs socket, waits for data, and generates an XML file which is then consumed by our Jenkins code (which I cannot share). I don't think this is the ultimate example, though. Because, in reality, there's many other ways to approach this. Ideas off the top of my head: a simple POST request on the brs side to a node-based HTTP server, or even saving results to the internal dev registry so it can be read by another brs app.

Ultimately speaking, we've been really looking forward to switch to Rooibos v5 here at Sky and NBCU, with at least 4 different projects currently using some form/fork of an earlier version specifically to support our various CI needs. Instead of forking v5, we'd like to contribute by providing mechanisms to make these integrations easier, whilst keeping it open enough for different purposes, implementations and setups. I don't think this PR is asking to support a socket connection, but rather expose the test results so others can adapt their own solutions on top. My project's solution just happens to be socket support because that's what we currently use, but any other approach would be perfectly valid, and much easier to implement with this work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also disregard my suggestion, it was merely a question out of curiosity, and it should work just fine as is.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luis-soares-sky ... thank you for sharing the socket code. I'm approving this pr, and I'll follow up to document how others can use it, when I get a free moment.

#222

I'll cut a release tomorrow.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luis-soares-sky/ @twig2let can we resolve the conflicts please?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one failing unit test, then we're all good.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/georgejecook/rooibos/actions/runs/4744047016/jobs/8424401801?pr=212

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a fix. It's not ideal but the BSConfig defined in the test isn't getting passed into the ProgramBuilder - it was commented out 13 months ago.

Tried passing the swv var into the ProgramBuilder but was getting different failures then.


'code coverage is not enabled in rooibos 4.0.0 - will get added again in future
' if Rooibos.Common.isFunction(RBS_reportCodeCoverage)
' 'bs:disable-next-line
Expand Down
3 changes: 2 additions & 1 deletion tests/bsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
"catchCrashes": true,
"lineWidth": 70,
"failFast": false,
"sendHomeOnFinish": false
"sendHomeOnFinish": false,
"keepAppOpen": true
},
"sourceMap": true
}