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

disable flaky ExpressionTests.testSubTypeTiming() #894 #1692

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

jukzi
Copy link
Contributor

@jukzi jukzi commented Jan 17, 2025

@jukzi jukzi requested a review from akurtakov January 17, 2025 09:24
@eclipse-platform-bot
Copy link
Contributor

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

runtime/tests/org.eclipse.core.expressions.tests/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch
From 912d4c2e7d44a9951842b8d917090c6be9565d8f Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Fri, 17 Jan 2025 09:30:22 +0000
Subject: [PATCH] Version bump(s) for 4.35 stream


diff --git a/runtime/tests/org.eclipse.core.expressions.tests/META-INF/MANIFEST.MF b/runtime/tests/org.eclipse.core.expressions.tests/META-INF/MANIFEST.MF
index d47738b787..69e2e7115c 100644
--- a/runtime/tests/org.eclipse.core.expressions.tests/META-INF/MANIFEST.MF
+++ b/runtime/tests/org.eclipse.core.expressions.tests/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.core.expressions.tests; singleton:=true
-Bundle-Version: 3.7.400.qualifier
+Bundle-Version: 3.7.500.qualifier
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
 Export-Package: 
-- 
2.47.1

Further information are available in Common Build Issues - Missing version increments.

Copy link
Contributor

Test Results

 1 755 files  ±0   1 755 suites  ±0   1h 27m 35s ⏱️ - 7m 35s
 4 170 tests ±0   4 147 ✅  - 1   23 💤 +1  0 ❌ ±0 
13 107 runs  ±0  12 940 ✅  - 3  167 💤 +3  0 ❌ ±0 

Results for commit aa87847. ± Comparison against base commit 2f18de7.

This pull request skips 1 test.
AllExpressionTests ExpressionTests ‑ testSubTypeTiming

@akurtakov
Copy link
Member

It doesn't look like there are any asserts that make sense for anything but the time measurement.
The test itself looks totally wrong as the "caching" version definitively can be slower on the first run as it populates the caches and even installs a BundleListener(!!!).
I would be in favor of totally removing the test after looking into it.

@jukzi
Copy link
Contributor Author

jukzi commented Jan 17, 2025

Locally running it works and performs a performancetest, so it could be used as such

@akurtakov
Copy link
Member

What I say is that even if fails locally that doesn't mean there is a problem. Please merge as is if you believe it's useful for smth.

@jukzi
Copy link
Contributor Author

jukzi commented Jan 17, 2025

i used it to verify the current implementation is faster then using Class::isAssignableFrom :-)

@jukzi jukzi merged commit 9e4e1ad into eclipse-platform:master Jan 17, 2025
14 of 17 checks passed
@jukzi jukzi deleted the testSubTypeTiming branch January 17, 2025 13:38
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.

3 participants