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

🧪 Ensure skipped test do not fail #2821

Merged
merged 7 commits into from
Jul 1, 2024

Conversation

nulrich
Copy link
Contributor

@nulrich nulrich commented Jun 20, 2024

Motivation

Some of our tests are skipped but have runtime issues. Jasmine considers them as skipped but DD Test Visibility as failed.

Changes

Ensuring that we do not execute the body of afterEach if we skipped the test in beforeEach.

Adding a Karma reporter to warn when we have failed skipped tests.

Testing

Test Visibility

image

I have gone over the contributing documentation.

Copy link

cit-pr-commenter bot commented Jun 20, 2024

Bundles Sizes Evolution

📦 Bundle Name Base Size Local Size 𝚫 𝚫% Status
Rum 160.25 KiB 160.25 KiB 0 B 0.00%
Logs 57.91 KiB 57.91 KiB 0 B 0.00%
Rum Slim 108.77 KiB 108.77 KiB 0 B 0.00%
Worker 25.21 KiB 25.21 KiB 0 B 0.00%
🚀 CPU Performance
Action Name Base Average Cpu Time (ms) Local Average Cpu Time (ms) 𝚫
addglobalcontext 0.002 0.001 -0.000
addaction 0.031 0.032 0.001
adderror 0.032 0.032 0.000
addtiming 0.001 0.001 0.000
startview 0.877 0.885 0.008
startstopsessionreplayrecording 0.844 0.827 -0.017
logmessage 0.020 0.019 -0.001
🧠 Memory Performance
Action Name Base Consumption Memory (bytes) Local Consumption Memory (bytes) 𝚫 (bytes)
addglobalcontext 21.57 KiB 19.68 KiB -1937 B
addaction 72.37 KiB 72.49 KiB 122 B
adderror 87.43 KiB 85.78 KiB -1694 B
addtiming 19.62 KiB 17.71 KiB -1953 B
startview 314.20 KiB 319.27 KiB 5.06 KiB
startstopsessionreplayrecording 16.82 KiB 15.54 KiB -1315 B
logmessage 67.43 KiB 70.33 KiB 2.90 KiB

🔗 RealWorld

@nulrich nulrich changed the title Ensure skipped test do not fail 🧪 Ensure skipped test do not fail Jun 21, 2024
@nulrich nulrich marked this pull request as ready for review June 21, 2024 08:23
@nulrich nulrich requested a review from a team as a code owner June 21, 2024 08:23
@@ -0,0 +1,20 @@
function KarmaSkippedFailedReporter(baseReporterDecorator, logger) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 praise: ‏Great!

Comment on lines 36 to 39
if (isIE()) {
return
}

Copy link
Member

@BenoitZugmeyer BenoitZugmeyer Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 suggestion: ‏Instead of duplicating the condition in afterEach (which is error-prone, as it can be forgotten), you could leverage registerCleanupTask in beforeEach and remove the afterEach usage. For example, in this test, we could write:

 beforeEach(() => {
    if (isIE()) {
      pending('no fetch support')
    }
    fetchStubManager = stubFetch()
    originalFetchStub = window.fetch
    requests = []
    requestsTrackingSubscription = initFetchObservable().subscribe((context) => {
      if (context.state === 'resolve') {
        requests.push(context)
      }
    })
    fetchStub = window.fetch as FetchStub
    registerCleanupTask(() => {
      requestsTrackingSubscription.unsubscribe()
      fetchStubManager.reset()
    })
  })

This could be a step toward https://datadoghq.atlassian.net/browse/RUM-5101

@nulrich nulrich merged commit c48a9bd into main Jul 1, 2024
20 checks passed
@nulrich nulrich deleted the nicolas.ulrich/fix-skipped-failed-tests branch July 1, 2024 12:35
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.

4 participants