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

Moving the SRA tests to a separate env #1272

Merged
merged 16 commits into from
Feb 6, 2019
Merged

Conversation

yfarjoun
Copy link
Contributor

@yfarjoun yfarjoun commented Feb 2, 2019

Description

Please explain the changes you made here.
Explain the motivation for making this change. What existing problem does the pull request solve?

Checklist

  • Code compiles correctly
  • New tests covering changes and new functionality
  • All tests passing
  • Extended the README / documentation, if necessary
  • Is not backward compatible (breaks binary or source compatibility)

@codecov-io

This comment has been minimized.

@yfarjoun
Copy link
Contributor Author

yfarjoun commented Feb 2, 2019

This should fix the breakage of the master branch.

@@ -7,6 +7,8 @@

import java.io.IOException;

//calling this an FTP test so that it could fail in travis
@Test(groups="ftp")
public class EnaRefServiceTest extends HtsjdkTest {
Copy link
Member

Choose a reason for hiding this comment

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

I don't like labelling this as an FTP test. Maybe instead of a new SRA block we can have an EXTERNAL_APIS test type that covers SRA and ENA.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Copy link
Member

@lbergelson lbergelson left a comment

Choose a reason for hiding this comment

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

@yfarjoun I think it's a good idea to group the ENA and SRA tests together if they're likely to fail together. Gross to have the ENA server test in FTP since that's just broken forever and has to be removed while the ENA test is potentially valuable.

build.gradle Outdated Show resolved Hide resolved
Copy link
Member

@lbergelson lbergelson left a comment

Choose a reason for hiding this comment

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

@yfarjoun One typo and then 👍 Thanks for doing this while I was incommunicado.

Co-Authored-By: yfarjoun <[email protected]>
@yfarjoun yfarjoun merged commit 52169ed into master Feb 6, 2019
@yfarjoun yfarjoun deleted the yf_allow_SRA_filures branch February 6, 2019 04:20
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