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

Added code coverage using mysql #29410

Closed
wants to merge 1 commit into from

Conversation

SergioBertolinSG
Copy link
Contributor

Currently running using mysql 5.7, after #29409 is merged it will run agains mysql 5.5.

@DeepDiver1975
Copy link
Member

codecov shows that mysql code paths are touched - nice - https://codecov.io/gh/owncloud/core/pull/29410/changes

Adding --group DB would give us some speed up here as well ...

@DeepDiver1975 DeepDiver1975 added this to the development milestone Nov 2, 2017
@SergioBertolinSG
Copy link
Contributor Author

Adding --group DB would give us some speed up here as well ...

I've added it in the last commit, is that ok?

if [[ "${DB_TYPE}" == "none" ]]; then
GROUP="--exclude-group DB"
else
GROUP="--group DB"
Copy link
Member

Choose a reason for hiding this comment

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

now sqlite is executed with --group DB as well - DB_TYPE none is missing

@SergioBertolinSG
Copy link
Contributor Author

SergioBertolinSG commented Nov 2, 2017

Some failures in drone after last commit:

1) Test\Encryption\EncryptionWrapperTest::testWrapStorage with data set #0 (true, array())
Your test case is not allowed to access the database.


/drone/src/tests/lib/TestCase.php:110
/drone/src/lib/composer/pimple/pimple/src/Pimple/Container.php:113
/drone/src/lib/private/AppFramework/Utility/SimpleContainer.php:111
/drone/src/lib/private/ServerContainer.php:87
/drone/src/lib/private/Server.php:1141
/drone/src/lib/private/AllConfig.php:90
/drone/src/lib/private/AllConfig.php:340
/drone/src/lib/private/AllConfig.php:251
/drone/src/lib/private/legacy/util.php:233
/drone/src/lib/private/Files/Filesystem.php:265
/drone/src/lib/private/Encryption/EncryptionWrapper.php:89
/drone/src/tests/lib/Encryption/EncryptionWrapperTest.php:75
2) Test\Files\External\PersonalMountTest::testFindByStorageId
Your test case is not allowed to access the database.

/drone/src/tests/lib/TestCase.php:110
/drone/src/lib/composer/pimple/pimple/src/Pimple/Container.php:113
/drone/src/lib/private/AppFramework/Utility/SimpleContainer.php:111
/drone/src/lib/private/ServerContainer.php:87
/drone/src/lib/private/Server.php:1141
/drone/src/lib/private/AllConfig.php:90
/drone/src/lib/private/AllConfig.php:340
/drone/src/lib/private/AllConfig.php:251
/drone/src/lib/private/legacy/util.php:233
/drone/src/lib/private/Files/Mount/Manager.php:124
/drone/src/tests/lib/Files/External/PersonalMountTest.php:47
3) Test\Files\Mount\ManagerTest::testFind
Your test case is not allowed to access the database.

/drone/src/tests/lib/TestCase.php:110
/drone/src/lib/composer/pimple/pimple/src/Pimple/Container.php:113
/drone/src/lib/private/AppFramework/Utility/SimpleContainer.php:111
/drone/src/lib/private/ServerContainer.php:87
/drone/src/lib/private/Server.php:1141
/drone/src/lib/private/AllConfig.php:90
/drone/src/lib/private/AllConfig.php:340
/drone/src/lib/private/AllConfig.php:251
/drone/src/lib/private/legacy/util.php:233
/drone/src/lib/private/Files/Mount/Manager.php:72
/drone/src/tests/lib/Files/Mount/ManagerTest.php:31
4) Test\Files\Mount\ManagerTest::testLong
Your test case is not allowed to access the database.

/drone/src/tests/lib/TestCase.php:110
/drone/src/lib/composer/pimple/pimple/src/Pimple/Container.php:113
/drone/src/lib/private/AppFramework/Utility/SimpleContainer.php:111
/drone/src/lib/private/ServerContainer.php:87
/drone/src/lib/private/Server.php:1141
/drone/src/lib/private/AllConfig.php:90
/drone/src/lib/private/AllConfig.php:340
/drone/src/lib/private/AllConfig.php:251
/drone/src/lib/private/legacy/util.php:233
/drone/src/lib/private/Files/Mount/Manager.php:124
/drone/src/tests/lib/Files/Mount/ManagerTest.php:64

@DeepDiver1975
Copy link
Member

Some failures in drone after last commit:

I'd say there are some annotations missing then

@DeepDiver1975 DeepDiver1975 mentioned this pull request Nov 2, 2017
9 tasks
@SergioBertolinSG
Copy link
Contributor Author

SergioBertolinSG commented Nov 2, 2017

Should I add

* @group DB

to all those tests?

@SergioBertolinSG
Copy link
Contributor Author

@DeepDiver1975 Can this be merged?

@SergioBertolinSG SergioBertolinSG self-assigned this Nov 3, 2017
@DeepDiver1975
Copy link
Member

@DeepDiver1975 Can this be merged?

no - the code coverage report is 💩 - the expectation is that the coverage increases when adding mysql - not to decrease

@SergioBertolinSG
Copy link
Contributor Author

Adding this commit f9500dc Reduced the coverage by 20,99%

Adding this one 849fcf7 incremented it 18,45%

Should it really appear incremented?

.drone.yml Outdated
@@ -80,14 +80,27 @@ pipeline:
matrix:
TEST_SUITE: coverage

codecov:
codecov_sqlite:
Copy link
Member

Choose a reason for hiding this comment

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

one step should be enough - the plugin can take multiple files

Copy link
Member

Choose a reason for hiding this comment

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

or even with wild cards

@SergioBertolinSG SergioBertolinSG force-pushed the drone-enabling-mysql-code-coverage branch from dc45fb1 to e1140dc Compare November 7, 2017 12:43
@SergioBertolinSG
Copy link
Contributor Author

Hmm flags appear in the report, but the github check doesn't split them

screen shot 2017-11-08 at 08 58 48

Do we need a different approach here? Making steps more explicit separating databases like in 794e529 ?

@SergioBertolinSG SergioBertolinSG force-pushed the drone-enabling-mysql-code-coverage branch 2 times, most recently from e5fc237 to 2badd16 Compare November 8, 2017 09:46
@PVince81 PVince81 modified the milestones: development, planned Nov 8, 2017
@SergioBertolinSG
Copy link
Contributor Author

Removed annotations in unit tests.

@owncloud owncloud deleted a comment from codecov bot Nov 8, 2017
@DeepDiver1975
Copy link
Member

@SergioBertolinSG please enable debug - https://github.com/robertstettner/drone-codecov#configuration
maybe we get some insights on how the plugin is uploading the reports

@codecov
Copy link

codecov bot commented Nov 8, 2017

Codecov Report

Merging #29410 into master will decrease coverage by 1.58%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #29410      +/-   ##
============================================
- Coverage     61.39%    59.8%   -1.59%     
- Complexity    17472    17477       +5     
============================================
  Files          1044     1044              
  Lines         57695    59249    +1554     
============================================
+ Hits          35419    35432      +13     
- Misses        22276    23817    +1541
Flag Coverage Δ Complexity Δ
#unit_tests 59.8% <ø> (?) 17477 <ø> (?)
Impacted Files Coverage Δ Complexity Δ
settings/templates/panels/admin/apps.php 6% <0%> (-94%) 0% <0%> (ø)
lib/private/App/CodeChecker/DeprecationCheck.php 9.52% <0%> (-90.48%) 5% <0%> (ø)
settings/templates/panels/admin/filesharing.php 10.25% <0%> (-89.75%) 0% <0%> (ø)
settings/templates/panels/personal/tokens.php 13.88% <0%> (-86.12%) 0% <0%> (ø)
.../federatedfilesharing/templates/settings-admin.php 21.42% <0%> (-78.58%) 0% <0%> (ø)
...deratedfilesharing/templates/settings-personal.php 23.25% <0%> (-76.75%) 0% <0%> (ø)
core/templates/404.php 25% <0%> (-75%) 0% <0%> (ø)
settings/templates/panels/admin/tips.php 25% <0%> (-75%) 0% <0%> (ø)
core/templates/403.php 25% <0%> (-75%) 0% <0%> (ø)
settings/templates/panels/admin/certificates.php 26.08% <0%> (-73.92%) 0% <0%> (ø)
... and 160 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8dba2ed...2347ac8. Read the comment docs.

@DeepDiver1975
Copy link
Member

Codecov upload has issues afaik. See the logs. Master Branch is used.

@SergioBertolinSG
Copy link
Contributor Author

Yes, adding it here for reference:

==> Uploading reports
    url: https://codecov.io
query: branch=master&commit=1f658a6c9808a5817cea716aacd9b36614be2e12&build=1064&build_url=&name=&tag=&slug=owncloud%2Fcore&yaml=.codecov.yml&service=&flags=unit_tests&pr=29410&job=

Here https://github.com/robertstettner/drone-codecov/blob/master/entrypoint.sh#L44-L45

the plugin uses DRONE_BRANCH.

In drone docs it says DRONE_BRANCH | commit branch

http://docs.drone.io/environment-reference/

But there is also a variable called DRONE_COMMIT_BRANCH there. Not sure if it is the same.

@DeepDiver1975
Copy link
Member

But there is also a variable called DRONE_COMMIT_BRANCH there. Not sure if it is the same.

@tboerger is there a way to see the actual env vars? THX

@SergioBertolinSG SergioBertolinSG force-pushed the drone-enabling-mysql-code-coverage branch from d694bef to 1ff8d93 Compare November 9, 2017 14:08
@SergioBertolinSG
Copy link
Contributor Author

Both DRONE_BRANCH and DRONE_COMMIT_BRANCH are the same : master.

This variable contains the current branch name:

DRONE_COMMIT_REFSPEC=drone-enabling-mysql-code-coverage:master

@SergioBertolinSG SergioBertolinSG force-pushed the drone-enabling-mysql-code-coverage branch from 1ff8d93 to 2347ac8 Compare November 10, 2017 08:33
@PVince81 PVince81 modified the milestones: planned, development Nov 22, 2017
@DeepDiver1975 DeepDiver1975 deleted the drone-enabling-mysql-code-coverage branch December 14, 2017 08:15
@lock
Copy link

lock bot commented Aug 1, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants