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

Add yarn Nexus cleanup task #343

Merged

Conversation

akhmanova
Copy link
Collaborator

@akhmanova akhmanova commented Nov 23, 2020

Remove temporary Nexus data for stale or
failed requests that uses "yarn" as package manager.

I am not really sure how to add a unit test, because for pip we have a magic number 42 there

Copy link

@athos-ribeiro athos-ribeiro left a comment

Choose a reason for hiding this comment

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

Can we add some unit tests to cover this change?

cachito/workers/tasks/yarn.py Outdated Show resolved Hide resolved
@athos-ribeiro
Copy link

athos-ribeiro commented Nov 23, 2020

I am not really sure how to add a unit test, because for pip we have a magic number 42 there

It would be similar. That "magic number" is just a fake request ID (and also the answer to the ultimate question of life, the universe, and everything)

@renanrodrigo
Copy link

I am not really sure how to add a unit test, because for pip we have a magic number 42 there

It would be similar. That "magic number" is just a fake request ID (and also the answer to the ultimate question of life, the universe, and everything)

Many people have speculated that if we knew exactly why the test had been written like that we would know a lot more about the nature of the Universe than we do now.

@akhmanova
Copy link
Collaborator Author

@athos-ribeiro Ok, there is a new unit test. But looks like something is fundamentally wrong with the universe and the test coverage is broken for me locally:

cachito/workers/tasks/yarn.py                    8      6    25%   7-19
--------------------------------------------------------------------------
TOTAL                                         3728   2893    22%
Coverage HTML written to dir htmlcov
Coverage XML written to file coverage.xml

@athos-ribeiro
Copy link

@athos-ribeiro Ok, there is a new unit test. But looks like something is fundamentally wrong with the universe and the test coverage is broken for me locally:

cachito/workers/tasks/yarn.py                    8      6    25%   7-19
--------------------------------------------------------------------------
TOTAL                                         3728   2893    22%
Coverage HTML written to dir htmlcov
Coverage XML written to file coverage.xml

Could that be that you are running just one specific test instead of the test suite?

Travis reposts 100% coverage there:

However, tests are failing:

E           AssertionError: expected call not found.

E           Expected: execute_script('js_cleanup', {'repository_name': 'some-yarn-repo-42', 'username': 'true-lover-of-douglas-adams'})

E           Actual: execute_script('js_cleanup', {'repository_name': 'cachito-yarn-42', 'username': 'cachito-yarn-42'})

Remove temporary Nexus data for stale or
failed requests that uses "yarn" as package manager.
Copy link

@athos-ribeiro athos-ribeiro left a comment

Choose a reason for hiding this comment

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

Nice work!

LGTM

@athos-ribeiro athos-ribeiro merged commit cf9d041 into containerbuildsystem:master Nov 25, 2020
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