Skip to content

Conversation

@nickvergessen
Copy link
Member

Integration tests are now only run when:

  • PHP was modified
  • a file in build/integration was modified
  • The commit is a merge commit

Signed-off-by: Joas Schilling <coding@schilljs.com>
@skjnldsv
Copy link
Member

Nice idea!!

@kesselb
Copy link
Collaborator

kesselb commented Aug 27, 2019

Integration tests are not relevant for pull requests like #16798?

@skjnldsv
Copy link
Member

Integration tests are not relevant for pull requests like #16798?

no, they're only testing apis :)

@nickvergessen
Copy link
Member Author

Integration tests are not relevant for pull requests like #16798?

No, integration tests make API calls only. No JS or browser involved.
Acceptance tests run the JS code

@kesselb
Copy link
Collaborator

kesselb commented Aug 27, 2019

No, integration tests make API calls only. No JS or browser involved.
Acceptance tests run the JS code

Thank you for explaining 👍

@blizzz
Copy link
Member

blizzz commented Aug 27, 2019

Mh, do I understand correctly, you want to run them only after a PR was merged?

@skjnldsv
Copy link
Member

Mh, do I understand correctly, you want to run them only after a PR was merged?

No, only if the commits include changes on a php file, an integration test or if it's a merge commit! :)

@nickvergessen
Copy link
Member Author

What @skjnldsv said. We do it like that since some weeks in Talk and it helped to improve the queue timers for JS/CSS only changes quite a lot. Now server is the biggest time drain and at least for pushes to JS/CSS only changes it is unnecessary to repeatedly check the integration tests.

Merge commits always run the integration tests on master/stableX afterwards

@rullzer
Copy link
Member

rullzer commented Aug 27, 2019

Could probably do the same on all the DB runs right?

@nickvergessen
Copy link
Member Author

Could probably do the same on all the DB runs right?

The time gain there is not a lot, but yeah. Anyway lets start like this and see how it works out.

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 27, 2019
@skjnldsv skjnldsv merged commit caf32d2 into master Aug 27, 2019
@skjnldsv skjnldsv deleted the techdebt/noid/only-run-integration-tests-on-php-changes branch August 27, 2019 16:12
@kesselb
Copy link
Collaborator

kesselb commented Aug 27, 2019

We could replace the default clone step with a custom one (https://docs.drone.io/user-guide/pipeline/cloning/#custom-logic) like for the submodules and run the check there.

Benefits:

  • Merge the submodule step with the clone step
  • Cache is not initialized

@rullzer rullzer mentioned this pull request Aug 28, 2019
@rullzer
Copy link
Member

rullzer commented Aug 28, 2019

/backport to stable16

@backportbot-nextcloud
Copy link

backport to stable16 in #16909

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish technical debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants