Skip to content

ingest now completes, fix assertions #6322#6324

Merged
kcondon merged 1 commit intodevelopfrom
6322-ingest-complete
Oct 29, 2019
Merged

ingest now completes, fix assertions #6322#6324
kcondon merged 1 commit intodevelopfrom
6322-ingest-complete

Conversation

@pdurbin
Copy link
Member

@pdurbin pdurbin commented Oct 29, 2019

partial solution for #6322

Here's how the output looks now, and I changed the assertions to match:

assert from line 337

{
    "status": "OK",
    "data": {
        "notifications": [
            {
                "id": 818,
                "type": "SUBMITTEDDS"
            },
            {
                "id": 816,
                "type": "INGESTCOMPLETED"
            },
            {
                "id": 815,
                "type": "SUBMITTEDDS"
            },
            {
                "id": 812,
                "type": "CREATEACC"
            }
        ]
    }
}

assert from line 389

{
    "status": "OK",
    "data": {
        "notifications": [
            {
                "id": 827,
                "type": "SUBMITTEDDS"
            },
            {
                "id": 825,
                "type": "SUBMITTEDDS"
            },
            {
                "id": 823,
                "type": "INGESTCOMPLETED"
            },
            {
                "id": 822,
                "type": "SUBMITTEDDS"
            },
            {
                "id": 819,
                "type": "CREATEACC"
            }
        ]
    }

I suppose this means that ingest is now succeeding while previously it was not. I'm not sure why. Here's the transition on phoenix where it flipped from passing to failing:

@coveralls
Copy link

Coverage Status

Coverage remained the same at 19.451% when pulling 0f79235 on 6322-ingest-complete into 4470369 on develop.

@pdurbin pdurbin mentioned this pull request Oct 29, 2019
@landreev landreev self-assigned this Oct 29, 2019
@landreev
Copy link
Contributor

I suppose this means that ingest is now succeeding while previously it was not.

My understanding is that the ingest was always succeeding - it's just that we now have a new ingest notification, that we didn't have before. (Was introduced in the PR merged last week - so this explains why the test started failing last week).

But the fix in this PR - adjust the expected notifications to assert - seems like the right thing to do.

@kcondon kcondon assigned kcondon and unassigned landreev Oct 29, 2019
@kcondon kcondon merged commit 62a21e2 into develop Oct 29, 2019
@kcondon kcondon deleted the 6322-ingest-complete branch October 29, 2019 19:44
@djbrooke djbrooke added this to the 4.18 milestone Oct 29, 2019
@pdurbin
Copy link
Member Author

pdurbin commented Oct 29, 2019

it's just that we now have a new ingest notification

Right, right, from pull request #6271. Thanks, @landreev

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.

5 participants