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

[TASK] Introduce Core-like testing with runTests.sh #327

Merged
merged 4 commits into from
Jun 3, 2024
Merged

Conversation

linawolf
Copy link
Collaborator

No description provided.

@linawolf linawolf marked this pull request as draft May 30, 2024 06:10
@linawolf linawolf force-pushed the task/tests branch 3 times, most recently from 1726b08 to 8e60acc Compare May 30, 2024 06:38
@linawolf linawolf marked this pull request as ready for review May 30, 2024 06:41
@linawolf linawolf requested a review from fsuter May 30, 2024 06:41
@linawolf
Copy link
Collaborator Author

@fsuter Please have a look if this is the direction that you imagined. I will add unit and functional tests later on. I am not sure if we have to drop nimut first.

@fsuter
Copy link
Contributor

fsuter commented Jun 2, 2024

Hi Lina. Thanks a lot for this huge work! Sorry for the late reply, I'm working only part-time these days.

For my understanding, I need to clarify a few things:

  • why is it needed to have a .ddev configuration? Is the "make" command not enough? Are you supposed to execute "make" from inside the container?
  • what's the "branch alias" introduced in the composer.json file? It does not correspond to an existing branch. External Import still uses "master" and there's no 7.2.x-dev branch. Is that coming from your fork?

As for the rest: 1) it's great to have this big code cleanup, 2) I don't understand all the details of the runTests.sh script, nor of the phpstan configuration, but I trust you on that.

@linawolf
Copy link
Collaborator Author

linawolf commented Jun 3, 2024

The ddev is not really needed and runTests.sh are executed in the docker containers provided by the script. I find it handy when debugging or instlalling stuff via composer as it comes with the correct PHP version. but I can exclude it locally from github.

The branch alias thingy allows me to require the current development version on dev-main as "^7.2.x-dev". This has the advantage I will notice once there is an official 7.2 Version and dev-main starts to develop for version 8.0 for example. The Core is doing it like this as well:
https://github.com/TYPO3/typo3/blob/main/typo3/sysext/core/composer.json#L103
It is not strictly required but comes in handy. I can remove it if you would rather not maintain development versions here.

@fsuter
Copy link
Contributor

fsuter commented Jun 3, 2024

Thanks for the explanations.

I would rather not have the DDEV configuration. As for the alias, I have never been that strict with branches. I used to created branches with funny names when starting new major versions, but this proved a hassle with the related extensions, so I'm, now just using "develop".

I forgot to answer one of your questions: for me it's ok to proceed with automated testing using nimut/testing-framework and change to typo3/testing-framework after the automation is in place.

@linawolf
Copy link
Collaborator Author

linawolf commented Jun 3, 2024

The branch-alias does not require any special naming requirements on branches. It is anlyzed by packagist whenever dev-main derived from your main branch is updated and will automatically make ^7.2.x-dev availible, without need for a a special branch or tag of that name. I can remove it if you want however.

linawolf added 3 commits June 3, 2024 10:27
Supplying a ddev configuration is not desired as tests can be run via the docker container configured by runTests.sh.

Contributors can use their own ddev configurations locally if desired.
@linawolf
Copy link
Collaborator Author

linawolf commented Jun 3, 2024

I removed the ddev configuration and added .ddev to the gitignore to document the desision.

@fsuter
Copy link
Contributor

fsuter commented Jun 3, 2024

The branch-alias does not require any special naming requirements on branches. It is anlyzed by packagist whenever dev-main derived from your main branch is updated and will automatically make ^7.2.x-dev availible, without need for a a special branch or tag of that name. I can remove it if you want however.

OK, that's fine then. But please note that I'm still using master and not main.

@linawolf
Copy link
Collaborator Author

linawolf commented Jun 3, 2024

Whatever default branch you have registered at packagist should be fine

@linawolf
Copy link
Collaborator Author

linawolf commented Jun 3, 2024

So are you ok with getting this merged? I have a 9 hour train ride today and could continue working on the functional tests...

@fsuter
Copy link
Contributor

fsuter commented Jun 3, 2024

So are you ok with getting this merged? I have a 9 hour train ride today and could continue working on the functional tests...

Yes, great, please proceed. Thanks a lot!

@linawolf linawolf merged commit 78372d6 into master Jun 3, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants