Cleanup CI actions

[aidan-casey]

After the major action refactor in #417, we have a couple of items to figure out here:

1. Do we want sub-splits to be published if tests are failing? If not, we should incorporate testing into the sub-split action.
2. Do we want to look into something like Symfony has for their isolated unit tests? Symfony runs these in parallel but within a single GitHub action using a fairly complex bash function. This function could likely be simplified for our use case.
3. Do we want to be testing isolated tests on Windows as well? Currently I am only running the integration tests on Windows.
4. Do we want to move coverage reporting to the isolated tests, capturing the metrics from unit tests or do we want to include integration tests which skew(?) these metrics. Further reading.

@Treggats / @brendt I welcome your feedback here.

[brendt]

• Would having individual GitHub actions for each component that are run on the individual repositories have a benefit?
• I don't have a strong opinion on how to split stuff, I just find it important that if something doesn't work, someone shouts at me before I tag stuff. I'd say that if tempest/framework is green for all actions, it should be safe to tag. So however things are split, tempest/framework should always paint the full picture

[aidan-casey]

Would having individual GitHub actions for each component that are run on the individual repositories have a benefit?

I don't think it really matters if they run here or on the other repos. Here probably gives us more visibility.

I don't have a strong opinion on how to split stuff, I just find it important that if something doesn't work, someone shouts at me before I tag stuff. I'd say that if tempest/framework is green for all actions, it should be safe to tag. So however things are split, tempest/framework should always paint the full picture

tempest/framework tests have their flaws though. Because it is autoloading all the packages, if one of the packages makes reference to another, but doesn't have it listed as a dependency, the tests will still work. Only the isolated testing (where we go to each package, install its dependencies, and then run its tests) shows us where that is at.

[Treggats]

Do we want sub-splits to be published if tests are failing? If not, we should incorporate testing into the sub-split action.

I'm not very familiar we the sub-split process, but in my mind this is the kind of thing that needs to be caught earlier. But in the case that's not possible, then we'd need to incorporate them.

Like run the tests before running the subsplit. Perhaps run the subsplit with a matrix, so they are not run sequentially. But I'm just spitballing here, haven't looked at the actual action.

Do we want to look into something like Symfony has for their isolated unit tests? Symfony runs these in parallel but within a single GitHub action using a fairly complex bash function. This function could likely be simplified for our use case.

At work I have this with our Dusk tests, no need for a complex bash script though. Just a matrix and different test suites configured.

Do we want to be testing isolated tests on Windows as well? Currently I am only running the integration tests on Windows.

I think that the isolated tests are covered by the integration tests, so in my mind I don't think it's needed.

Do we want to move coverage reporting to the isolated tests, capturing the metrics from unit tests or do we want to include integration tests which skew(?) these metrics

Looked into the referenced website, I think that test coverage is better but maybe harder to accomplish. But for complex functionality code coverage could be valuable.

On to your question, probably best to scope the code coverage to the unit tests.

tempest/framework tests have their flaws though. Because it is autoloading all the packages, if one of the packages makes reference to another, but doesn't have it listed as a dependency, the tests will still work. Only the isolated testing (where we go to each package, install its dependencies, and then run its tests) shows us where that is at.

Basically just checking the dependency stuff?

[aidan-casey]

I'm not very familiar we the sub-split process, but in my mind this is the kind of thing that needs to be caught earlier.

I agree. Let's try without for now and see the implications.

...no need for a complex bash script though. Just a matrix and different test suites configured.

The purpose of the complex bash script was more to prevent tons of jobs spawning. For example, right now we have 51 jobs and that is without running the isolated tests on Windows. The Symfony script runs only one per platform/stability matrix. The biggest benefit of this is avoiding GitHub's queueing.

I think that the isolated tests are covered by the integration tests, so in my mind I don't think it's needed.

Generally I agree.

I think that test coverage is better but maybe harder to accomplish.

The way I look at it, it's almost impossible to verify your test coverage is adequate without having complete code coverage. As the article says:

While test coverage can create the image of a high-quality codebase, it does not guarantee that the code is bug-free.

Test coverage is only as adequate as the quality of the tests: Incorrect or incomplete tests may not identify errors.

This leads me to agree with your conclusion on scoping coverage to unit tests.

Basically just checking the dependency stuff?

Kind of? Say, for example, we committed the following to the console package:

{
    "name": "tempest/console",
    "description": "The PHP framework that gets out of your way.",
    "license": "MIT",
    "minimum-stability": "dev",
    "require": {
        "php": "^8.3",
        "tempest/core": "dev-main",
-        "tempest/container": "dev-main",
        "tempest/highlight": "^2.0",
        "tempest/log": "dev-main",
        "tempest/reflection": "dev-main",
        "tempest/support": "dev-main",
        "tempest/validation": "dev-main",
        "ext-readline": "*"
    },
    "autoload": {
        "psr-4": {
            "Tempest\\Console\\": "src"
        }
    },
    "bin": [
        "bin/tempest"
    ]
}

The tempest/framework tests would actually pass, because the root autoload is able to load those classes from the monorepo. The tempest/console isolated tests would fail, because it would change into that directory, install the composer dependencies, and then run the unit tests. This already caught some problems we had with missing dependencies.

It's worth noting, Symfony only tests packages in isolation except for the integration tests they do. I envision a similar setup here.

[brendt]

tempest/framework tests have their flaws though. Because it is autoloading all the packages, if one of the packages makes reference to another, but doesn't have it listed as a dependency, the tests will still work. Only the isolated testing (where we go to each package, install its dependencies, and then run its tests) shows us where that is at.

Makes sense.

The purpose of the complex bash script was more to prevent tons of jobs spawning. For example, right now we have 51 jobs and that is without running the isolated tests on Windows. The Symfony script runs only one per platform/stability matrix. The biggest benefit of this is avoiding GitHub's queueing.

I like separate actions because you get a more detailed overview of what's wrong, but I don't have a strong opinion about it.

It's worth noting, Symfony only tests packages in isolation except for the integration tests they do. I envision a similar setup here.

Makes sense. Although let's first set this up, and then worry about improving coverage over time. If we wanna get everything right from the get go, we'll just end up nowhere.

[Treggats]

Makes sense. Although let's first set this up, and then worry about improving coverage over time. If we wanna get everything right from the get go, we'll just end up nowhere.

I agree, these are things that can be updated over time. Making new improvements all the time, not make one very big one which can be slowed down because of one little issue.

[Treggats]

The tempest/framework tests would actually pass, because the root autoload is able to load those classes from the monorepo. The tempest/console isolated tests would fail, because it would change into that directory, install the composer dependencies, and then run the unit tests. This already caught some problems we had with missing dependencies.

I think it's a nice addition to the sub-split workflow. Perhaps as a dependency to it, so that the subsplit doesn't happen if there is an issue with composer (and maybe the tests)