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 tests to drone and remove travis #803

Merged
merged 9 commits into from Nov 21, 2018
Merged

Add tests to drone and remove travis #803

merged 9 commits into from Nov 21, 2018

Conversation

dpakach
Copy link
Contributor

@dpakach dpakach commented Nov 14, 2018

Fixes: #790
Licence: MIT or AGPL

Description

Add tests to drone and add make targets for running tests and remove travis.

Features

Screenshots or screencasts

Caveats

Tests

Test plan

Tested on

  • Windows/Firefox
  • Android 6.1/Chrome

TODO

  • ...
  • ...

Check list

  • Code is properly documented
  • Code is properly formatted
  • Commits have been squashed
  • Tests (unit, integration, api and/or acceptance) are included
  • Documentation (manuals or wiki) has been updated or is not required

Reviewers

@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/gallery/3/21
There are code style errors reported, so this PR will also need a commit that fixes the code style.

@phil-davis
Copy link
Contributor

phil-davis commented Nov 14, 2018

There is some problem with the make file:
https://drone.owncloud.com/owncloud/gallery/3/28

+ make test-php-unit
Makefile:137: warning: overriding recipe for target 'vendor'
Makefile:50: warning: ignoring old recipe for target 'vendor'
make: *** No rule to make target 'vendor/bin/phpunit', needed by 'test-php-unit'. Stop.

maybe there is some old vendor stuff still there, and/or other new lines that still need pasting in.

@dpakach dpakach force-pushed the add-drone branch 9 times, most recently from 08a8425 to 09ed452 Compare November 15, 2018 10:07
@codecov
Copy link

codecov bot commented Nov 15, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@14be0e3). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #803   +/-   ##
=========================================
  Coverage          ?   55.09%           
  Complexity        ?      353           
=========================================
  Files             ?       48           
  Lines             ?     1630           
  Branches          ?        0           
=========================================
  Hits              ?      898           
  Misses            ?      732           
  Partials          ?        0

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 14be0e3...bd67579. Read the comment docs.

@phil-davis
Copy link
Contributor

@dpakach I played with this last night. It seems that:

exclude('c3.php')

actually does not exclude the file from php-cs-fixer - the exclude() method is for directories.
In other repos that have this we seem to have got lucky, and maybe c3.php does not exist anyway.

There are 2 Symfony Finder methods:
notName('c3.php') - I think that will exclude an file called c3.php anywhere.
notPath('/^c3.php/') - should exclude any path that starts with ```c3.php- which will just exclude the top-levelc3.php`` file.

Along the way I made other changes trying to cleanup the existing php-cs-fixer from the vendor directory - maybe I succeeded?

@phil-davis
Copy link
Contributor

The drone acceptance test jobs say:

php vendor/bin/codecept run acceptance --env chrome
Codeception PHP Testing Framework v2.5.0
Powered by PHPUnit 5.7.27 by Sebastian Bergmann and contributors.
Running with seed: 



Time: 1.23 seconds, Memory: 18.00MB

No tests executed! 

The setup looks similar to on Travis, but using Chrome instead of Firefox.
That needs to be looked at.

The unit and integration test counts are the same in drone as Travis and all pass.

@dpakach
Copy link
Contributor Author

dpakach commented Nov 16, 2018

@phil-davis There are only some acceptance tests in this app and the way they are written is very different from what we have been doing in other apps.

@phil-davis
Copy link
Contributor

@dpakach yes, it is a different test framework. There are only 2 actual acceptance test cases/scenarios.

You could spend a little time trying to run the acceptance tests locally - hopefully you just do the commands locally that the Travis YML does!? Then see if that might give any clue about what is missing in the drone setup. e.g. try to run with Chrome and maybe something needs setting up for that or?

@individual-it
Copy link
Member

still No tests executed!

@dpakach dpakach changed the title Add tests to drone Add tests to drone and remove travis Nov 20, 2018
@phil-davis
Copy link
Contributor

@DeepDiver1975 this needs the repo to be set to "drone required" and "Travis not required"

@PVince81 PVince81 merged commit 625502e into master Nov 21, 2018
@PVince81 PVince81 deleted the add-drone branch November 21, 2018 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants