Skip to content

Quick fix for a failing harvesting test #8937#8939

Merged
pdurbin merged 2 commits intodevelopfrom
8937-harvest-test-failure
Aug 26, 2022
Merged

Quick fix for a failing harvesting test #8937#8939
pdurbin merged 2 commits intodevelopfrom
8937-harvest-test-failure

Conversation

@landreev
Copy link
Contributor

What this PR does / why we need it:

Which issue(s) this PR closes:

Closes #8937

Special notes for your reviewer:

Suggestions on how to test this:

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?:

Additional documentation:

@coveralls
Copy link

coveralls commented Aug 26, 2022

Coverage Status

Coverage remained the same at 20.045% when pulling 781ca74 on 8937-harvest-test-failure into 1ef73b0 on develop.

@pdurbin pdurbin self-assigned this Aug 26, 2022
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

Looks like a doubling, as discussed. I'm waiting for the Jenkins tests to pass before I merge this.

@landreev
Copy link
Contributor Author

Before increasing that sleep, I ran the test a few times and it was failing about 50% of the time. After doubling it I ran it a few more times and wasn't able to get it to fail.
So, looks like a solid fix. 😄

@landreev
Copy link
Contributor Author

I'll take a look to see why it bombed once I'm in. ☹️

@landreev
Copy link
Contributor Author

This is kind of insane that the harvesting test is still bombing. So maybe it's not the duration after all. (not sure yet what's up with the other test that failed)

@landreev
Copy link
Contributor Author

Oh. Was that the wrong sleep interval to increase? - I.e., it may be failing not on account of the set create taking too long, but because we try to create the set before the publish command has completed on the dataset there...

@pdurbin pdurbin added this to the 5.12 milestone Aug 26, 2022
@landreev
Copy link
Contributor Author

landreev commented Aug 26, 2022

OK, the tests have passed.
The other test that failed during the first run - it died in a really random place, with a 500 from an attempt to destroy a dataset at the end of a test. I feel like dismissing this one as a fluke.

@pdurbin pdurbin merged commit eed4775 into develop Aug 26, 2022
@pdurbin pdurbin deleted the 8937-harvest-test-failure branch August 26, 2022 19:33
@pdurbin
Copy link
Member

pdurbin commented Aug 26, 2022

Before merging I confirmed that indeed all API tests passed. Again, all we're doing is bumping up the amount of sleep.

@landreev
Copy link
Contributor Author

Yeah, I edited the comment in the issue, documenting that it wasn't the doubling of the 5 sec. sleep, but an extra sleep added before that step.

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.

test failure in develop: HarvestingServerIT.testOaiFunctionality

3 participants