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

[MIG] sentry: Migrated to 15.0 #2376

Closed
wants to merge 20 commits into from

Conversation

airlessproject
Copy link

No description provided.

naglis and others added 20 commits July 19, 2022 00:57
* [ADD] sentry module

* [FIX] updated sentry module according to PR comments
- [FIX] sentry: fixes missing `raven` library preventing loading of modules
- [FIX] 2to3 script on py file
- [FIX] add requirements.txt
sentry: It is not always possible to read commit information from
a production environment. In those cases it is useful to be able
to set a release version manually.

[UPD] Update sentry.pot
[UPD] Update sentry.pot
Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-tools-14.0/server-tools-14.0-sentry
Translate-URL: https://translation.odoo-community.org/projects/server-tools-14-0/server-tools-14-0-sentry/

[UPD] README.rst

[UPD] Update sentry.pot
The following warning is fixed:

    DeprecationWarning: Using or importing the ABCs from 'collections' instead of
    from 'collections.abc' is deprecated since Python 3.3, and in 3.9 it will stop
    working
Allow using `sentry_release` or `sentry_odoo_dir` in the Odoo
configuration file.

Previously, the `sentry_odoo_dir` was never actually respected. It would
always be overridden by `sentry_release`. Even if `sentry_release` is
not set, it will use an empty value instead of using `sentry_odoo_dir`
to find the Git commit hash.

After this commit, the `sentry_release` parameter still takes
precedence. However, if `sentry_release` is not set and
`sentry_odoo_dir` is set, then `sentry_odoo_dir` will be used to find
the appropriate Git commit hash, which will be used as the `release`
value.

Both cases are covered by the added unit tests.
@aisopuro
Copy link
Contributor

It looks like code coverage is failing due to some lines in the configuration reading not being covered by tests.

I don't know the policy regarding these kinds of things: are some of these lines OK to skip? I wouldn't know how to set up tests that would cover them.

@extrememicro you've contributed here recently: do you know where the author might find guidelines regarding how to do coverage for lines that are in __init__.py files?

@airlessproject
Copy link
Author

@traviswaelbro please check previous comment by @aisopuro, maybe you can help as well, considering you migrated the module to 14.0 and made other improvements?

@traviswaelbro
Copy link
Contributor

I've not worked with code coverage at all, but you may be able to mock out import errors. You can also mock out return exceptions to trigger the except clause.

I'm not sure how you can tweak the Odoo configuration for some of the other complaints from coverage.

@aisopuro
Copy link
Contributor

aisopuro commented Aug 2, 2022

Introducing mocking just to avoid coverage errors seems a little overkill IMO. In my estimation this is confuration-reading code that is difficult to test with unit tests like this. I would be fine with putting in # pragma: no cover comments in order to get this PR moving.

@traviswaelbro
Copy link
Contributor

I'd agree with that

@aisopuro aisopuro mentioned this pull request Aug 10, 2022
@aisopuro
Copy link
Contributor

aisopuro commented Aug 10, 2022

@traviswaelbro I made a new PR with fixes to tests that resolved the coverage failures: #2386. I've kept all of @airlessproject commits so they're credited properly.

@thomaspaulb
Copy link
Contributor

Replaced by #2386

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.