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

feat: implement fixmeinci modifier to conditionally skip tests in CI environments #35106

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Sharma-IT
Copy link

Overview

This PR introduces a new test modifier .fixmeinci() which allows tests to be conditionally skipped in CI environments while still running normally in local development environments. This feature helps address flaky or environment-specific tests that may not be reliable in CI pipelines but are still valuable for local testing.

Features

  • New .fixmeinci() modifier for test cases and test suites
  • Conditional test skipping based on CI environment detection
  • Support for test-level and suite-level annotations
  • Chaining support for combining with other modifiers
  • Comprehensive test coverage

Implementation Details

1. Added Type Definitions and Interfaces

  • Extended the RunnableType definition in timeoutManager.ts to include the new fixmeinci type
  • Updated the Modifier type in test.ts to include fixmeinci as a valid modifier type

2. Test and Suite-Level Implementations

  • Added test.fixmeinci() method with support for chaining (e.g., test.fixmeinci.skip())
  • Added test.describe.fixmeinci() for suite-level skipping
  • Implemented proper handling of annotations based on environment

3. Runtime Logic

  • Modified processAnnotation function in workerMain.ts to:
    • Skip processing annotations entirely when not in CI
    • Convert fixmeinci annotations to fixme annotations in CI environments
  • Updated suite modifier processing to ensure consistent behavior

4. CI Environment Detection

  • Implemented consistent checking of process.env.CI using !!process.env.CI to handle various truthy values
  • Added safeguards against improper environment variable handling

5. Test Coverage

  • Created comprehensive test file (test-fixmeinci.spec.ts) with scenarios for:
    • Individual test cases with fixmeinci
    • Test suites with fixmeinci
    • Mixing fixmeinci with other modifiers
    • Verifying local vs CI behavior

Code Examples

Test Case Usage

// Skip individual test only in CI
test('flaky test', async ({ page }) => {
  test.fixmeinci('Flaky in CI pipeline');
  // Test will run locally but be skipped in CI
});

// With chaining
test.fixmeinci('skips in CI only', async ({ page }) => {
  // Test will be skipped in CI but run locally
});

Suite Usage

// Skip an entire suite in CI
test.describe.fixmeinci('suite of environment-sensitive tests', () => {
  test('test 1', async () => {
    // This test will be skipped in CI
  });
  
  test('test 2', async () => {
    // This test will also be skipped in CI
  });
});

// Or use within a suite
test.describe('suite with some CI-sensitive tests', () => {
  test.fixmeinci(); // All tests in this suite will be skipped in CI
  
  test('sensitive test', async () => {
    // Will be skipped in CI
  });
});

Testing Strategy

Tests cover both positive and negative scenarios:

  • Verification that tests are properly skipped in CI environments
  • Verification that tests run normally in local environments
  • Verification that test expectations match actual behavior
  • Testing proper annotation handling for both environments

Benefits

  1. Improved Test Reliability: Allows flagging tests that are known to be environment-sensitive
  2. Better Developer Experience: Tests still run locally where they can be fixed and verified
  3. Clean CI Pipelines: Prevents flaky tests from causing false CI failures
  4. Discoverable Syntax: Follows the pattern of existing modifiers like test.skip() and test.fixme()

Future Considerations

  • Further enhancements could include conditional testing based on other environment variables
  • Consider adding this feature to the public documentation

Documentation Update

Update to the internal documentation to reflect the new modifier:

### test.fixmeinci([condition], [description])

Mark a test or suite as skipped when running in CI environments, but run it normally in local environments.

- When `process.env.CI` is truthy, the test behaves like `test.fixme()`
- When `process.env.CI` is falsy, the test runs normally

This is useful for tests that are unreliable in CI environments but still valuable in local development.

Related Issues

This feature helps address common issues with flaky tests in CI pipelines without removing test coverage for local development.

…environments

This commit introduces the new \`.fixmeinci()\` modifier that:

- Allows marking tests to be skipped only in CI environments while running normally locally
- Works with both test cases (\`test.fixmeinci()\`) and test suites (\`test.describe.fixmeinci()\`)
- Provides proper chaining support (\`test.fixmeinci.skip()\`, \`test.fixmeinci.only()\`, etc.)
- Can be used with optional reason messages (\`test.fixmeinci('reason')\`)

Implementation details:

- Added new type definitions in timeoutManager.ts
- Extended the TestType interface to include the fixmeinci modifier
- Modified the annotation processing to handle fixmeinci annotations differently in CI vs local
- Ensured annotations are only added in CI environments, not in local environments
- Added comprehensive test coverage to validate behavior in both environments

This implementation helps address flaky or environment-specific tests by providing
a clean way to skip them in CI pipelines while still enforcing they run correctly
in local development environments.
@Sharma-IT
Copy link
Author

@microsoft-github-policy-service agree

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Mar 9, 2025

Test results for "tests 1"

4 flaky ⚠️ [firefox-page] › tests/page/page-evaluate.spec.ts:403:3 › should throw for too deep reference chain @firefox-ubuntu-22.04-node18
⚠️ [playwright-test] › tests/ui-mode-trace.spec.ts:341:5 › should work behind reverse proxy @macos-latest-node18-1
⚠️ [chromium-library] › tests/library/client-certificates.spec.ts:400:3 › browser › should not hang on tls errors during TLS 1.2 handshake @ubuntu-22.04-chromium-tip-of-tree
⚠️ [chromium-library] › tests/library/video.spec.ts:379:5 › screencast › should capture navigation @ubuntu-22.04-chromium-tip-of-tree

38676 passed, 803 skipped
✔️✔️✔️

Merge workflow run.

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.

1 participant