Skip to content

Only run integration tests when PHP was modified - #16879

Merged
skjnldsv merged 1 commit into
masterfrom
techdebt/noid/only-run-integration-tests-on-php-changes
Aug 27, 2019
Merged

Only run integration tests when PHP was modified#16879
skjnldsv merged 1 commit into
masterfrom
techdebt/noid/only-run-integration-tests-on-php-changes

Conversation

@nickvergessen

Copy link
Copy Markdown
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
Copy Markdown
Member

Nice idea!!

@kesselb

Copy link
Copy Markdown
Contributor

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

@skjnldsv

Copy link
Copy Markdown
Member

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

no, they're only testing apis :)

@nickvergessen

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
Contributor

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
Copy Markdown
Member

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

@skjnldsv

Copy link
Copy Markdown
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
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

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

@nickvergessen

Copy link
Copy Markdown
MemberAuthor

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.

@skjnldsvskjnldsv 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 masterAug 27, 2019
@skjnldsv
skjnldsv deleted the techdebt/noid/only-run-integration-tests-on-php-changes branch August 27, 2019 16:12
@kesselb

Copy link
Copy Markdown
Contributor

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

@rullzerrullzer mentioned this pull request Aug 28, 2019
@rullzer

Copy link
Copy Markdown
Member

/backport to stable16

@backportbot-nextcloud

Copy link
Copy Markdown

backport to stable16 in #16909

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

Labels

4. to releaseReady to be released and/or waiting for tests to finishtechnical debt🧱 🤔🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@nickvergessen@skjnldsv@kesselb@blizzz@rullzer@tobiasKaminsky