Skip to content

Zend/tests: organize some tests with sub directories - #15638

Merged
Girgias merged 3 commits into
php:masterfrom
DanielEScherzer:test-organization
Oct 13, 2024
Merged

Zend/tests: organize some tests with sub directories#15638
Girgias merged 3 commits into
php:masterfrom
DanielEScherzer:test-organization

Conversation

@DanielEScherzer

Copy link
Copy Markdown
Member

Move some low-hanging fruit, creating new directories for the tests for

  • access modifiers
  • class_alias()
  • constant expressions
  • constructor property promotion
  • __debugInfo()
  • dereferencing
  • first class callable syntax

Additionally, move some tests into the existing sub directory for closure-related tests

Work towards GH-15631

@DanielEScherzer

Copy link
Copy Markdown
Member Author

@Girgias can I get your thoughts on this? Having recently had to scroll through the list of Zend tests a number of times for other recent patches, this would really come in handy

@Girgias

Girgias commented Sep 22, 2024

Copy link
Copy Markdown
Member

I am overall in favour of this sort of changes as needing to run all the Zend tests when just checking one specific part is very time-consuming, and does lead to wasted CI time when one thinks one has run all the tests but in reality no.

Comment thread Zend/tests/debug_info/debug_info-error-0.0.phpt Outdated
Move some low-hanging fruit, creating new directories for the tests for

* access modifiers
* `class_alias()`
* constant expressions
* constructor property promotion
* `__debugInfo()`
* dereferencing
* first class callable syntax

Additionally, move some tests into the existing sub directory for
closure-related tests

Work towards phpGH-15631
@DanielEScherzer

Copy link
Copy Markdown
Member Author

Not sure why GitHub was reporting a merge conflict, git rebase master worked perfectly

@Girgias
Girgias merged commit 8475d5f into php:master Oct 13, 2024
@Girgias

Girgias commented Oct 13, 2024

Copy link
Copy Markdown
Member

Thank you. :)

@DanielEScherzer
DanielEScherzer deleted the test-organization branch October 13, 2024 19:05
@TimWolla

Copy link
Copy Markdown
Member

Since this PR we have both:

https://github.com/php/php-src/tree/master/Zend/tests/constexpr

and

https://github.com/php/php-src/tree/master/Zend/tests/constant_expressions

which is probably not a good idea. For #17213 (which should land at the end of the week), I've now opted for Zend/tests/first_class_callable/constexpr/. But Zend/tests/constexpr/first_class_callable/attributes.phpt (with constexpr and first_class_callable reversed) would also work.

@DanielEScherzer

Copy link
Copy Markdown
Member Author

Since this PR we have both:

https://github.com/php/php-src/tree/master/Zend/tests/constexpr

and

https://github.com/php/php-src/tree/master/Zend/tests/constant_expressions

which is probably not a good idea. For #17213 (which should land at the end of the week), I've now opted for Zend/tests/first_class_callable/constexpr/. But Zend/tests/constexpr/first_class_callable/attributes.phpt (with constexpr and first_class_callable reversed) would also work.

Sorry about that - based on the names I assumed that they referred to two different contexts, but I guess not. Do you want me to merge those directories?

@TimWolla

Copy link
Copy Markdown
Member

Do you want me to merge those directories?

Please double-check that they are indeed referring to the same thing and then merge them, yes. Keep the constexpr one, it's the older one.

DanielEScherzer added a commit to DanielEScherzer/php-src that referenced this pull request Feb 21, 2025
While reviewing the existing tests in the `constexpr` directory, I found that
some of the names were not updated to reflect the contents when the contents
were changed in php#9301.

Follow-up to php#15638
Girgias pushed a commit that referenced this pull request Feb 21, 2025
…es (#17872)

While reviewing the existing tests in the `constexpr` directory, I found that
some of the names were not updated to reflect the contents when the contents
were changed in #9301.

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

Labels

None yet

3 participants