Repository: oppia/oppia
File: scripts/servers.py (Python)

Code before:
    server gracefully. If the suite_name is not in the list of the
    acceptance tests suite names, then raises an exception.

    Args:
        suite_name: str. The suite name whose tests should be run.
        headless: bool. Whether to run the acceptance tests in headless mode.
        mobile: bool. Whether to run the acceptance tests in mobile mode.
        prod_env: bool. Whether to run the acceptance tests in production mode.
        stdout: int. The file descriptor where the standard output of the 
            subprocess is sent.

    Yields:
        psutil.Process. The jest testing process.

    Raises:
        Exception. The suite_name is not in the list of the acceptance tests
            suite names.
    """
    available_suites = {}
    with open(
        common.ACCEPTANCE_TEST_CONFIG_FILE_PATH, 'r', encoding='utf-8') as f:
        filedata = json.load(f)
        for suite_type in filedata:
            suites = filedata[suite_type]
            for suite in suites:
                available_suites[suite['name']] = suite['module']
    if suite_name not in available_suites:
        raise Exception('Invalid suite name: %s' % suite_name)

    os.environ['HEADLESS'] = 'true' if headless else 'false'
    os.environ['MOBILE'] = 'true' if mobile else 'false'
    os.environ['SPEC_NAME'] = suite_name
    os.environ['PROD_ENV'] = 'true' if prod_env else 'false'

    nodemodules_jest_bin_path = os.path.join(
        common.NODE_MODULES_PATH, '.bin', 'jest')

    acceptance_tests_args = [
        nodemodules_jest_bin_path,
        '%s' % os.path.join(available_suites[suite_name]),
        '--config=./core/tests/puppeteer-acceptance-tests/jest.config.js'
    ]

    # OK to use shell=True here because we are passing string literals,
    # and verifying that the passed suite-name are within the list of
    # the suites we have, so there is no risk of a shell-injection attack.
    managed_acceptance_tests_proc = managed_process(
        acceptance_tests_args,
        human_readable_name='Acceptance Tests Server',
        shell=True,
        raise_on_nonzero_exit=False,

Review comment (suggestion):
This can be simplified by using `for suites in filedata.values()`. Also, it might be cleaner with a dictionary comprehension, but that's optional

Code after:
    server gracefully. If the suite_name is not in the list of the
    acceptance tests suite names, then raises an exception.

    Args:
        suite_name: str. The suite name whose tests should be run.
        headless: bool. Whether to run the acceptance tests in headless mode.
        mobile: bool. Whether to run the acceptance tests in mobile mode.
        prod_env: bool. Whether to run the acceptance tests in production mode.
        stdout: int. The file descriptor where the standard output of the 
            subprocess is sent.

    Yields:
        psutil.Process. The jest testing process.

    Raises:
        Exception. The suite_name is not in the list of the acceptance tests
            suite names.
    """
    available_suites = {}
    with open(
        common.ACCEPTANCE_TEST_CONFIG_FILE_PATH, 'r', encoding='utf-8'
    ) as f:
        filedata = json.load(f)
        for suites in filedata.values():
            for suite in suites:
                available_suites[suite['name']] = suite['module']
    if suite_name not in available_suites:
        raise Exception('Invalid suite name: %s' % suite_name)

    os.environ['HEADLESS'] = 'true' if headless else 'false'
    os.environ['MOBILE'] = 'true' if mobile else 'false'
    os.environ['SPEC_NAME'] = suite_name
    os.environ['PROD_ENV'] = 'true' if prod_env else 'false'

    nodemodules_jest_bin_path = os.path.join(
        common.NODE_MODULES_PATH, '.bin', 'jest')

    acceptance_tests_args = [
        nodemodules_jest_bin_path,
        '%s' % os.path.join(available_suites[suite_name]),
        '--config=./core/tests/puppeteer-acceptance-tests/jest.config.js'
    ]

    # OK to use shell=True here because we are passing string literals,
    # and verifying that the passed suite-name are within the list of
    # the suites we have, so there is no risk of a shell-injection attack.
    managed_acceptance_tests_proc = managed_process(
        acceptance_tests_args,
        human_readable_name='Acceptance Tests Server',
        shell=True,
        raise_on_nonzero_exit=False,