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

Configure runs unit tests #174

Merged
merged 10 commits into from
Oct 13, 2020
Merged

Configure runs unit tests #174

merged 10 commits into from
Oct 13, 2020

Conversation

evmiguel
Copy link
Contributor

@evmiguel evmiguel commented Oct 5, 2020

No description provided.

@evmiguel evmiguel force-pushed the configure-runs-unit-tests branch from 01e9f22 to 1eb1b58 Compare October 5, 2020 22:01
Copy link
Contributor

@rmeritz rmeritz left a comment

Choose a reason for hiding this comment

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

I left some feedback to help guide the future work.

@evmiguel evmiguel force-pushed the configure-runs-unit-tests branch 2 times, most recently from 18228aa to 1352804 Compare October 6, 2020 16:45
@evmiguel evmiguel marked this pull request as ready for review October 6, 2020 21:04
@evmiguel evmiguel requested a review from rmeritz October 6, 2020 21:15
@evmiguel evmiguel force-pushed the configure-runs-unit-tests branch from 90a3ea6 to 97da2ea Compare October 6, 2020 21:52
Copy link
Contributor

@rmeritz rmeritz left a comment

Choose a reason for hiding this comment

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

Just some random detail oriented comments.

@evmiguel evmiguel force-pushed the configure-runs-unit-tests branch from f7efab6 to 9e0b125 Compare October 7, 2020 18:28
@spectranaut
Copy link
Contributor

Thx for all this! the one additional test I'd like to see is:

const activeRuns = await RunService.configureRuns({
    test_version_id: testVersion.id,
    apg_example_ids: [apgExample.id],
    at_browser_pairs: [
        {
            at_name_id: at.at_name_id,
            at_version: atVersionNumber,
            browser_id: browser.id,
            browser_version: browserVersionNumber
        }
        {
            at_name_id: at.at_name_id,
            // The only difference between this at_browser_pair and the one above is that the atVersionNumber is different!
            // We want to allow two different runs that are exactly the same other than a version number, in case there are still
            // "runs" for an older AT that need to be moved from "draft" to "final" while a new version of the AT is being tested.
            at_version: atVersionNumber2,
            browser_id: browser.id,
            browser_version: browserVersionNumber
        }

    ],
    run_status_id: runStatus.id
});

@evmiguel evmiguel force-pushed the configure-runs-unit-tests branch from 30b3204 to 2c11bfd Compare October 7, 2020 18:50
run_status_id: runStatus.id
});

const activeRuns = await RunService.configureRuns({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spectranaut here's the test!

@evmiguel evmiguel force-pushed the configure-runs-unit-tests branch from 3dc5c84 to 49b7d89 Compare October 7, 2020 21:08
Copy link
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

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

It all looks good to me!

@evmiguel evmiguel merged commit ea26723 into main Oct 13, 2020
@evmiguel evmiguel deleted the configure-runs-unit-tests branch October 13, 2020 15:04
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

Successfully merging this pull request may close these issues.

3 participants