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

[enh] Add unit/functional tests for apps #779

Merged
merged 6 commits into from Sep 17, 2019

Conversation

@alexAubin
Copy link
Member

commented Aug 17, 2019

The problem

We don't have tests for apps '-'

Solution

Implement some ...

PR Status

Tested on my machine, but still need to implement at least 3 of them

I think I spotted an important issue related to LDAP group permissions related to failed remove, gotta investigate

How to test

Run the tests using ynh-dev or pytest manually

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :
@Josue-T

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

I think I spotted an important issue related to LDAP group permissions related to failed remove, gotta investigate

Do you have any log. Maybe I could help to investigate...

@decentral1se
Copy link
Member

left a comment

Documentation addition to the README on how to run these?

They need to be run in the context of ynh-dev / live yunohost or?

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2019

Do you have any log. Maybe I could help to investigate...

It's not clear to me how to reproduce ... but basically some times I ended up with the check_ldap_db_consistency failing because some remaining permission existed for an app that was supposed to be uninstalled ... That might be because I was tweaking a lot of code and the remove operation was sometime failing because before the part where it removes the thing from ldap.

So maybe I'll just wrap stuff in try/except to avoid that kind of things in the future even though that seems to happen mainly during dev.

Documentation addition to the README on how to run these?
They need to be run in the context of ynh-dev / live yunohost or?

Yes, in the context of a post-installed yunohost. I use ./ynh-dev test yunohost/apps to run them, (c.f. here) or you can run pytest by hand

@alexAubin alexAubin added this to the 3.7.x milestone Sep 14, 2019

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Sep 14, 2019

Proposing to merge soonish though I will implement the last tests for #769

@alexAubin alexAubin force-pushed the tests-for-apps branch from c08b5d9 to 9249b99 Sep 15, 2019

@alexAubin alexAubin force-pushed the tests-for-apps branch from 9249b99 to aa3687b Sep 15, 2019

@alexAubin alexAubin changed the base branch from stretch-unstable to moar-sanity-check-for-app-operations Sep 15, 2019

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Sep 15, 2019

So that was painful ... but I finally implemented and ran the remaining tests which allowed to find bugs in #769 and #785 so we're good to go

The whole triptic of PR is a mess : this one is based on #785 itself based on #769 so i temporarily changed the base branch to this one is based on #785 in case you want to read the relevant commit of this PR easily...

@alexAubin alexAubin changed the base branch from moar-sanity-check-for-app-operations to stretch-unstable Sep 17, 2019

@alexAubin alexAubin merged commit 0436c16 into stretch-unstable Sep 17, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@alexAubin alexAubin deleted the tests-for-apps branch Sep 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.