Skip to content

Conversation

@violoncelloCH
Copy link
Member

@violoncelloCH violoncelloCH commented Jan 2, 2019

for #24

@MorrisJobke that's what I created...

  • .travis.yml is adapted from bookmarks' one
  • .drone.yml is the one from apps complemented with parts of user_saml's one

hope that's how they should look like and it's working this way

cc @nextcloud/user_external

@violoncelloCH violoncelloCH added enhancement New feature or request 3. to review labels Jan 2, 2019
@violoncelloCH violoncelloCH added this to the 0.5 milestone Jan 2, 2019
@violoncelloCH
Copy link
Member Author

ohh, it looks like travis is already building something

@violoncelloCH
Copy link
Member Author

I removed the composer install because we don't have a composer.json with dependencies, right?

@violoncelloCH
Copy link
Member Author

ant test wanted a build.xml so I took the one from the apps repo

However I'm not sure if we really need ant test (coming from the old apps .travis.yml) if we already have the other tests.
For phpunit in the scripts of .travis.yml, the parameter --configuration phpunit.xml is passed. We don't (yet) have a phpunit.xml. Do we need one or does phpunit run without the parameter?

@MorrisJobke
Copy link
Member

If you push a new commit to this branch then the drone job should also start ;)

@violoncelloCH
Copy link
Member Author

The drone sign-off-check on PRs complains about the github_token (The environment variable GITHUB_TOKEN has no proper value.). I don't know if I've done something wrong with the environment variable or if my token I've set in drone settings has not the right permissions. Could you take a look at it @MorrisJobke ?

@violoncelloCH
Copy link
Member Author

violoncelloCH commented Jan 4, 2019

for travis:
travis is now failing where I expect it to fail.

  • The builds for master don't proceed because we don't yet support NC 16, so I'm not sure if I should rather remove the tests on master or add support for 16?
  • The builds on stable15 fail because of occ check-code -- see occ check-code compliance #30
    However I'm not sure if there might be other problems coming after this one is fixed.

@MorrisJobke
Copy link
Member

The drone sign-off-check on PRs complains about the github_token (The environment variable GITHUB_TOKEN has no proper value.). I don't know if I've done something wrong with the environment variable or if my token I've set in drone settings has not the right permissions. Could you take a look at it @MorrisJobke ?

That should be handled by the DCO bot. @rullzer Could you add it here?

@MorrisJobke
Copy link
Member

travis is now failing where I expect it to fail.

  • The builds for master don't proceed because we don't yet support NC 16, so I'm not sure if I should rather remove the tests on master or add support for 16?
  • The builds on stable15 fail because of occ check-code -- see occ check-code compliance #30
    However I'm not sure if there might be other problems coming after this one is fixed.

Then for now: comment out the elements in the matrix that runs it on master (re-enable it in a PR that fixes this) and also disable the check-code for now. AFAIK we don't execute it anyways on install as of now.

@violoncelloCH
Copy link
Member Author

violoncelloCH commented Jan 7, 2019

That should be handled by the DCO bot.

does this mean I should remove the signed-off-check from drone?

@violoncelloCH
Copy link
Member Author

violoncelloCH commented Jan 7, 2019

where went the travis build now? DCO is here (thank you @rullzer), but travis disappeared somehow and doesn't build the new commits

@violoncelloCH violoncelloCH modified the milestones: 0.5, 0.5.1 Jan 7, 2019
@MorrisJobke
Copy link
Member

does this mean I should remove the signed-off-check from drone?

Correct-

@MorrisJobke
Copy link
Member

where went the travis build now? DCO is here (thank you @rullzer), but travis disappeared somehow and doesn't build the new commits

Good question ... the hook was properly delivered :/

@MorrisJobke
Copy link
Member

Create another commit and push again, maybe it works then.

@violoncelloCH
Copy link
Member Author

Create another commit and push again, maybe it works then.

hmm, doesn't seem to work

@violoncelloCH
Copy link
Member Author

"branches" and "build history" in travis are also empty like...

@violoncelloCH
Copy link
Member Author

whohoo, it works again... I commented out the branches: only: - master as well because I thought this wouldn't be needed with all master builds commented out, but somehow this seemed to be the problem...
now let's see if the build completes...

@violoncelloCH
Copy link
Member Author

For phpunit in the scripts of .travis.yml, the parameter --configuration phpunit.xml is passed. We don't (yet) have a phpunit.xml. Do we need one or does phpunit run without the parameter?

now this is the point where travis fails...
I'll probably just try what happens if I remove the parameter... If we need the parameter I can re-add it at any time...

@violoncelloCH
Copy link
Member Author

okay, that doesn't work
hmm, I'll need some help, sorry... I've no experience with these php unit tests...
The files we have in test/ here look quite different from what I can find in other nextcloud apps... I'll read something about unit testing, but I don't know if I can do this...

@MorrisJobke
Copy link
Member

okay, that doesn't work
hmm, I'll need some help, sorry... I've no experience with these php unit tests...
The files we have in test/ here look quite different from what I can find in other nextcloud apps... I'll read something about unit testing, but I don't know if I can do this...

Use this as configuration.xml: https://github.com/nextcloud/apps/blob/master/build.xml

At least this should be a good start. And put the configuration inside the tests directory to make excluding it for build purposes easier. ;)

@violoncelloCH
Copy link
Member Author

thank you very much for your help! @MorrisJobke
phpunit now exits with 0, but says that no tests were executed...
the ones with php coverage fail with Notifying that no code coverage data is available for repository "g/nextcloud/user_external" and revision "2099f41964381b91f1a90b5d7c6bed799851ff78"... Failed { "message": "Not found", "description": "The requested page was not found, or you have no access to view it." }

@violoncelloCH
Copy link
Member Author

@MorrisJobke should I comment out the phpunit tests so we can merge this without them? So we at least have the other tests and can try to get the unit tests to work later... what do you think?

@MorrisJobke
Copy link
Member

@MorrisJobke should I comment out the phpunit tests so we can merge this without them? So we at least have the other tests and can try to get the unit tests to work later... what do you think?

Go for that - sorry - don't have time to dig into this.

@violoncelloCH
Copy link
Member Author

@MorrisJobke okay, will do that... no problem! thank you for your support!

violoncelloCH and others added 18 commits February 27, 2019 20:30
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
as we don't support 14

Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
…t.xml`

Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Co-Authored-By: violoncelloCH <22591354+violoncelloCH@users.noreply.github.com>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
Signed-off-by: Jonas Sulzer <jonas@violoncello.ch>
@violoncelloCH
Copy link
Member Author

rebased on master and re-enabled drone check-app-compatibility test
please review

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@violoncelloCH violoncelloCH merged commit 929c42c into master Feb 27, 2019
@violoncelloCH violoncelloCH deleted the ci_travis_drone branch February 27, 2019 22:18
This was referenced Feb 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review enhancement New feature or request help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants