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

test: add unit tests #72

Merged
merged 71 commits into from
Apr 29, 2023
Merged

test: add unit tests #72

merged 71 commits into from
Apr 29, 2023

Conversation

deliaBlue
Copy link
Collaborator

@deliaBlue deliaBlue commented Apr 19, 2023

Closes #52

In order to add a test for filter_multimappers.py the following changes had been made:

  • Create the test
  • Create the test files (input and output files)
  • modify the script to stop if the file is empty and fix count_indels and find_best_alignments calls
  • Add dependencies in both the environment.yml and environment.dev.yml
  • Add test in CI.yml
  • Add coverage file

@deliaBlue deliaBlue requested a review from uniqueg April 19, 2023 11:00
@deliaBlue deliaBlue linked an issue Apr 19, 2023 that may be closed by this pull request
@zavolanlab zavolanlab deleted a comment from codecov-commenter Apr 19, 2023
Iris Mestres added 2 commits April 19, 2023 13:40
@zavolanlab zavolanlab deleted a comment from codecov-commenter Apr 19, 2023
@zavolanlab zavolanlab deleted a comment from codecov-commenter Apr 25, 2023
Copy link
Member

@uniqueg uniqueg left a comment

Choose a reason for hiding this comment

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

Oh, I'm really sorry @deliaBlue. I had reviewed this on Saturday, and was wondering why you didn't respond. But now I've seen that I hadn't actually submitted the review. Sorry about that 🙈

environment.dev.yml Show resolved Hide resolved
.github/workflows/CI.yml Outdated Show resolved Hide resolved
.coverage Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
.github/workflows/CI.yml Outdated Show resolved Hide resolved
.github/workflows/CI.yml Outdated Show resolved Hide resolved
scripts/tests/test_filter_multimappers.py Outdated Show resolved Hide resolved
Copy link
Member

@uniqueg uniqueg left a comment

Choose a reason for hiding this comment

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

See individual comments.

Copy link
Member

@uniqueg uniqueg left a comment

Choose a reason for hiding this comment

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

Sorry for adding a whole bunch of new comments 🙈

scripts/filter_multimappers.py Outdated Show resolved Hide resolved
scripts/filter_multimappers.py Outdated Show resolved Hide resolved
scripts/filter_multimappers.py Outdated Show resolved Hide resolved
scripts/filter_multimappers.py Outdated Show resolved Hide resolved
.github/workflows/CI.yml Outdated Show resolved Hide resolved
@deliaBlue
Copy link
Collaborator Author

Regarding the token option you mention here, If you open the codecov link in the CI, you will see the warning unusable report. If you click on download you will see this:

<Error>
<Code>ExpiredToken</Code>
<Message>The provided token has expired.</Message>
<Details>
Request signature expired at: 2023-04-27T09:14:19+00:00
</Details>
</Error>

I thought that maybe adding the token it would fix this; it did not. I'll take a look at it later.

@uniqueg uniqueg changed the title test: add test for filter_multimapper.py test: unit tests for filter_multimapper.py Apr 27, 2023
@uniqueg
Copy link
Member

uniqueg commented Apr 27, 2023

Regarding the token option you mention here, If you open the codecov link in the CI, you will see the warning unusable report. If you click on download you will see this:

<Error>
<Code>ExpiredToken</Code>
<Message>The provided token has expired.</Message>
<Details>
Request signature expired at: 2023-04-27T09:14:19+00:00
</Details>
</Error>

I thought that maybe adding the token it would fix this; it did not. I'll take a look at it later.

But did you actually try without a token? Whenever I saw this the token was there. The thing is that I think the tokens are scoped to each repository, so you can't use an organization-scoped CodeCov token. And indeed, such a token is not defined:

image

So I suggest that you first try without a token, and if/when it turns out that we really need a token, I will create one on CodeCov and set it here as a repository secret (I don't think that you have the permissions to do that, but you can try).

@deliaBlue
Copy link
Collaborator Author

I try it without the token and still obtain the same.
Also, as you can see, I tried to "fix the path" adding the correct one on the codecov.yml as explained here but still obtain the "unusable report" error and in fact, when checking the "unusable report" the path to the coverage file is not changed, I'm not quite sure the error is in the path but I cannot argue any other option; I cannot see where the problem is.

@deliaBlue
Copy link
Collaborator Author

I'll proceed on deleting the coverage call. This issue can be addressed in #74 .

@uniqueg uniqueg changed the title test: unit tests for filter_multimapper.py test: add unit tests Apr 29, 2023
@uniqueg uniqueg merged commit e5f282e into dev Apr 29, 2023
@uniqueg uniqueg deleted the 52-unit-tests-filter_multimapperpy branch April 29, 2023 01: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.

create unit tests for filter_multimapper.py
3 participants