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

[2.x] Adds skipOnWindows(), skipOnMac(), and skipOnLinux() #757

Merged
merged 6 commits into from Mar 31, 2023

Conversation

nunomaduro
Copy link
Member

@nunomaduro nunomaduro commented Mar 31, 2023

The addition made in this pull request is the skipOnWindows(), skipOnMac(), and skipOnLinux() methods, which allows users to bypass a particular test no those environments.

test('example', function () {
    expect(true)->toBeTrue();
})->skipOnWindows();

It's quite typical for open-source developers to have a specific group of tests that aren't compatible with Windows. Therefore, having this "skip" feature integrated into the core of the framework could prove to be beneficial.

@nunomaduro nunomaduro changed the title [2.x] Adds skipOnWindows() [2.x] Adds skipOnWindows(), skipOnMac(), and skipOnLinux() Mar 31, 2023
@nunomaduro nunomaduro merged commit d21ae25 into 2.x Mar 31, 2023
40 checks passed
@nunomaduro nunomaduro deleted the feat/skip-on-windows branch March 31, 2023 22:43
@arturluizbr
Copy link

I think it would be better to tag tests and skip the tagged tests during execution. This way, people can choose to skip certain tests based on their specific needs.

Something like this:

test('example', function () {
    expect(true)->toBeTrue();
})->usingTags('windows','aws','mysql');

@nunomaduro
Copy link
Member Author

That's groups? https://pestphp.com/docs/grouping-tests

@dkarvounaris
Copy link

dkarvounaris commented Apr 2, 2023

Sorry, I commented furiously on Twitter instead of coming here for the discussion... hey it's Sunday after all :D

While using Pest v2 on a new project, I noticed a need for something similar and realized that having the equivalents of runOnWindowsOnly(), runOnMacOnly(), and runOnLinuxOnly() or a shorter version like runOn<OS_FAMILY>() could be more beneficial for tests than using the skip-versions. Explicitly including the systems on which the tests should run feels like it would provide greater control, over an implicit inclusion of the skip-methods.

For instance, if you were to use the runOn<OS_FAMILY>() versions and called one of these methods, the tests would be skipped for any other system where it was not explicitly included. However, if you chained more than one runOn<OS_FAMILY>(), the tests would then run on those additional systems as well.

The difference may seem slightly, but usually, explicitly defining things in software development is better because it provides greater control and transparency, leaves less open to interpretation and can avoid unforeseen issues or bugs.

Although the skip-versions can exclude known-failing tests, they do not account for untested systems that may also fail. On the other hand, the runOn-methods provide greater control and allow for more specific cases, depending on things we know more certainly. Since most PHP code specific to a system (at least what I've seen) is written in this way, it's safer in these cases to depend on things known, than on things not known. Additionally, the runOn-methods don't limit us to defining only one system or several while being explicit. Anything not defined as runOn<OS_FAMILY>(), would be skipped, so they would provide everything the skip-methods would plus a greater control to be more strict or explicit.

Considering that, if the skip-methods were to be included, the feature would seem incomplete without at least a skipOnUnknown() method. Even then, it would still not provide the same level of control as the runOn-methods.

Also, compared to groups, it would provide more value as the simple skipping can already be achieved by using groups as @arturluizbr suggested - just that it's not "automatic". The runOn-Methods would allow for scenarios not achievable with groups.

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.

None yet

4 participants