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

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

Merged
merged 3 commits into from
Oct 13, 2024

Conversation

DanielEScherzer
Copy link
Contributor

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

Girgias commented Sep 22, 2024

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.

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
Contributor 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
10 checks passed
@Girgias
Copy link
Member

Girgias commented Oct 13, 2024

Thank you. :)

@DanielEScherzer DanielEScherzer deleted the test-organization branch October 13, 2024 19:05
@TimWolla
Copy link
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
Contributor 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
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.

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.

Organize Zend/tests/*.phpt tests into subdirectories for easier navigation
3 participants