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

fix resubmission tests #78

Merged
merged 1 commit into from
May 26, 2023

Conversation

bernt-matthias
Copy link
Contributor

on the 2nd submission the env variables get lost: see galaxyproject/galaxy#9747 .. galaxyproject/galaxy#12852

For some reason bash evaluates this as successful even if the if statement crashes. so actually there are only 2 submissions (1 resubmission), i.e. multiple resubmissions still do not work.

Even if the tests are not executed at the moment I would suggest to fix them by doing a "real OOM" resubmission and treat the case of a lost variable as error.

on the 2nd submission the env variables get lost. For some reason
bash evaluates this as successful even if the if statement crashes.
so actually there are only 2 submissions (1 resubmission), i.e.
multiple resubmissions still do not work.

Hence I suggest to to a "real OOM" resubmission and treat the case
of a lost variable as error.
@coveralls
Copy link
Collaborator

coveralls commented Jan 15, 2023

Pull Request Test Coverage Report for Build 3923746443

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 96.714%

Totals Coverage Status
Change from base Build 3913663123: -0.2%
Covered Lines: 918
Relevant Lines: 941

💛 - Coveralls

@nuwang
Copy link
Member

nuwang commented Jan 19, 2023

@bernt-matthias Thanks for making this PR. @cat-bro and I were discussing this issue, and ultimately ,because resubmission doesn't work with dynamic destinations, it looks like we would need some kind of fix in Galaxy for things to work at an acceptable level. One possibility would be to somehow remove the cache in resubmits only, and recompute the destination afresh. Is that your understanding as well?

Copy link
Member

@nuwang nuwang left a comment

Choose a reason for hiding this comment

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

@bernt-matthias Thanks for fixing this test. I'm hoping something can be fixed on the Galaxy end at some point, but till then I'll rebase the existing tests for activating resubmit against this.

@nuwang nuwang merged commit 6e91232 into galaxyproject:main May 26, 2023
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.

None yet

3 participants