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

Organize Zend/tests/*.phpt tests into subdirectories for easier navigation #15631

Open
DanielEScherzer opened this issue Aug 29, 2024 · 7 comments · Fixed by #15638
Open

Organize Zend/tests/*.phpt tests into subdirectories for easier navigation #15631

DanielEScherzer opened this issue Aug 29, 2024 · 7 comments · Fixed by #15638

Comments

@DanielEScherzer
Copy link
Contributor

Description

I wanted to take a look at any of the enum-related tests, but on GitHub navigating to https://github.com/php/php-src/tree/master/Zend/tests shows

Sorry, we had to truncate this directory to 1,000 files. 1,534 entries were omitted from the list. Latest commit info may be omitted.

I propose that the tests that are directly in the top-level Zend/tests/*.phpt be organized a bit with sub directories so that they can be navigated more easily, both on GitHub and in downloaded copies of PHP. For an easy example, there are 21 different tests for creating class aliases, class_alias_001.phpt through class_alias_021.phpt - these could be grouped together in their own directory (probably just named class_alias).

I'm happy to send the patches to work on this reorganization if there is buy-in, but figured I should ask before I started the work. Since this doesn't affect the actual execution of PHP I didn't think it was applicable to propose on the internals list, so adding it here.

PS: this is tagged as a feature but only because that was the most applicable of the GitHub issue forms, but it isn't really a feature

@iluuu1994
Copy link
Member

Hi @DanielEScherzer. That will be a pain in the butt when merging bug fixes upward. Not sure I'd be in favor.

@DanielEScherzer
Copy link
Contributor Author

Hi @DanielEScherzer. That will be a pain in the butt when merging bug fixes upward.

I hadn't thought about that. But, looking through all 8.2 commits since the start of the month, I only count 6 commits that modify or delete existing test files (rather than adding new files, which shouldn't cause problems) and all 6 are for extensions (7878a2c, 1b52ecd, 4d71580, 2829065, 6713d51, 11fbe88). In fact, I had to go all the way back to your May 6 commit f8d1864 to find an 8.2 commit that affected existing Zend test files, and the most recent for 8.3 that I could find was also from you that day, 5aa5080

My understanding from looking through the git history is that normally bug fixes only result in adding new tests, since changing existing tests suggests that the fix also modifies some stable behavior?

@iluuu1994
Copy link
Member

rather than adding new files, which shouldn't cause problems

Not problems, per-se. But if the idea is to split the tests, then they probably no longer belong in root and will have to be moved again.

@DanielEScherzer
Copy link
Contributor Author

rather than adding new files, which shouldn't cause problems

Not problems, per-se. But if the idea is to split the tests, then they probably no longer belong in root and will have to be moved again.

My proposal was just to split the tests enough to be navigable on GitHub, I'm not suggesting to create a strict hierarchy - we would still most likely end up with a bunch of tests in the top level. How about I send a demo patch to show what I'm thinking?

@cmb69
Copy link
Member

cmb69 commented Aug 29, 2024

Modifying the directory structure is always a bit of an issue, not only for merging, but also for git log and git blame etc. However, if we have proper git renames, it's probably not that bad.

Maybe we should start moving all bug*.phpt and bug* directories to a bug/ directory, and do the same for gh*. Especially the former would reduce the number of toplevel dirents considerably. And if occassionally a file is not moved into the right directory when merging up, that's not a big deal anyway.

DanielEScherzer added a commit to DanielEScherzer/php-src that referenced this issue Aug 29, 2024
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

Just wanting to note that there is https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view to allow ignoring some commits via git blame, that might satisfy the concerns

I sent #15638 as a demonstration of what I was thinking - while I agree that having a top-level bug/ would probably help, that is a case where there might be more conflicts merging up, so I didn't want to start there

@cmb69 cmb69 linked a pull request Aug 29, 2024 that will close this issue
DanielEScherzer added a commit to DanielEScherzer/php-src that referenced this issue Sep 26, 2024
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 added a commit to DanielEScherzer/php-src that referenced this issue Oct 9, 2024
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
Girgias pushed a commit that referenced this issue Oct 13, 2024
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 subdirectory for
closure-related tests

Work towards GH-15631
DanielEScherzer added a commit to DanielEScherzer/php-src that referenced this issue Oct 13, 2024
Move more low-hanging fruit, creating new directories for the tests for

* comparisons
* dynamic calls
* error messages
* `error_reporting()`
* exceptions
* `foreach()`
* garbage collection
* group `use` statements
* heredoc and nowdoc
* `goto` jumps
* late static binding
* magic methods
* namespaces
* numeric literal separators
* objects
* oss-fuzz casse
* `settype()`
* cleaning of temporary values
* `unset()`

Additionally, move some tests into the existing sub directory for `list()`
tests.

Also fix some test numbers in the names of the `goto` tests.

Work towards phpGH-15631
@DanielEScherzer
Copy link
Contributor Author

@cmb69 I see that you linked my PR as closing this issue - it was merely intended as a first step, https://github.com/php/php-src/tree/master/Zend/tests still reports

Sorry, we had to truncate this directory to 1,000 files. 1,370 entries were omitted from the list. Latest commit info may be omitted.

Would you mind re-opening this issue? I don't seem to have the rights to do so.
After #16423 merges I'll go through the whole set of bug*.phpt and gh*.phpt tests and try and organize those into existing sub directories, since that seems to be where the vast majority of the remaining tests are concentrated

DanielEScherzer added a commit to DanielEScherzer/php-src that referenced this issue Oct 13, 2024
Move more low-hanging fruit, creating new directories for the tests for

* comparisons
* dynamic calls
* error messages
* `error_reporting()`
* exceptions
* `foreach()`
* garbage collection
* group `use` statements
* heredoc and nowdoc
* `goto` jumps
* late static binding
* magic methods
* namespaces
* numeric literal separators
* objects
* oss-fuzz cases
* `settype()`
* cleaning of temporary values
* `unset()`

Additionally, move some tests into the existing sub directory for `list()`
tests.

Also fix some test numbers in the names of the `goto` tests.

Work towards phpGH-15631
@iluuu1994 iluuu1994 reopened this Oct 13, 2024
Girgias pushed a commit that referenced this issue Oct 14, 2024
Move more low-hanging fruit, creating new directories for the tests for:

* comparisons
* dynamic calls
* error messages
* `error_reporting()`
* exceptions
* `foreach()`
* garbage collection
* group `use` statements
* heredoc and nowdoc
* `goto` jumps
* late static binding
* magic methods
* namespaces
* numeric literal separators
* objects
* `settype()`
* cleaning of temporary values
* `unset()`

Additionally, move some tests into the existing subdirectory for `list()`
tests.

Drive-by fixes of some test numbers in the names of the `goto` tests.

Work towards GH-15631
DanielEScherzer added a commit to DanielEScherzer/php-src that referenced this issue Oct 14, 2024
First pass at moving `Zend/tests/bug*` tests to existing sub directories

Work towards phpGH-15631
DanielEScherzer added a commit to DanielEScherzer/php-src that referenced this issue Nov 28, 2024
First pass at moving `Zend/tests/bug*` tests to existing sub directories

Work towards phpGH-15631
DanielEScherzer added a commit to DanielEScherzer/php-src that referenced this issue Feb 9, 2025
First pass at moving `Zend/tests/bug*` tests to existing sub directories

Work towards phpGH-15631
Girgias pushed a commit that referenced this issue Feb 10, 2025
First pass at moving `Zend/tests/bug*` tests to existing sub directories

Work towards GH-15631
DanielEScherzer added a commit to DanielEScherzer/php-src that referenced this issue Feb 13, 2025
First pass at moving `Zend/tests/gh*` tests to existing sub directories

Work towards phpGH-15631
Girgias pushed a commit that referenced this issue Feb 13, 2025
First pass at moving `Zend/tests/gh*` tests to existing sub directories

Work towards GH-15631
DanielEScherzer added a commit to DanielEScherzer/php-src that referenced this issue Feb 14, 2025
Second pass through `Zend/tests/bug*` to organize the tests.

Move tests to existing sub directories, and create some new sub directories:
* `ArrayAccess`
* `autoload`
* `clone`
* `serialize` (also covers `unserialize()`)
* `switch`

Work towards phpGH-15631
Girgias pushed a commit that referenced this issue Feb 14, 2025
Second pass through `Zend/tests/bug*` to organize the tests.

Move tests to existing sub directories, and create some new sub directories:
* `ArrayAccess`
* `autoload`
* `clone`
* `serialize` (also covers `unserialize()`)
* `switch`

Work towards GH-15631
DanielEScherzer added a commit to DanielEScherzer/php-src that referenced this issue Feb 15, 2025
Move more tests into existing directories

Work towards phpGH-15631
Girgias pushed a commit that referenced this issue Feb 15, 2025
Move more tests into existing directories

Work towards GH-15631
DanielEScherzer added a commit to DanielEScherzer/php-src that referenced this issue Feb 16, 2025
Create a new subdirectory for tests relating to inheritance

Work towards phpGH-15631
Girgias pushed a commit that referenced this issue Feb 16, 2025
Create a new subdirectory for tests relating to inheritance

Work towards GH-15631
DanielEScherzer added a commit to DanielEScherzer/php-src that referenced this issue Feb 21, 2025
Create new sub directories for tests related to backtraces and for tests
related to `$this` being reserved in different places and not being usable or
reassignable.

Work towards phpGH-15631
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants