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 problematic example #10642

Merged
merged 2 commits into from
Jul 23, 2024
Merged

Disable problematic example #10642

merged 2 commits into from
Jul 23, 2024

Conversation

hubertp
Copy link
Collaborator

@hubertp hubertp commented Jul 23, 2024

Some jobs appear to be stuck because of it.

@hubertp hubertp added the CI: No changelog needed Do not require a changelog entry for this PR. label Jul 23, 2024
@hubertp hubertp added the CI: Ready to merge This PR is eligible for automatic merge label Jul 23, 2024
@radeusgd
Copy link
Member

I don't think this PR does what it says it does.

@radeusgd radeusgd removed the CI: Ready to merge This PR is eligible for automatic merge label Jul 23, 2024
@radeusgd
Copy link
Member

radeusgd commented Jul 23, 2024

Unless I am greatly mistaken, the main method is only ran when manually running the GC_Example.enso script file.

To actually disable the test, you need to do the change elsewhere:

diff --git a/test/Base_Tests/src/Runtime/Managed_Resource_Spec.enso b/test/Base_Tests/src/Runtime/Managed_Resource_Spec.enso
--- a/test/Base_Tests/src/Runtime/Managed_Resource_Spec.enso	(revision a4259d31e0ea025aae4e50bbc8533f07ee536708)
+++ b/test/Base_Tests/src/Runtime/Managed_Resource_Spec.enso	(date 1721744396649)
@@ -58,7 +58,7 @@
         r_3 = Panic.recover Any <| Managed_Resource.bracket 42 (_-> Nothing) (_-> Panic.throw "action")
         r_3.catch . should_equal "action"
 
-    group_builder.specify "allocate lots of resources at once" <|
+    group_builder.specify "allocate lots of resources at once" pending=(if Environment.get "CI" . is_nothing . not then "GC Example is marked as pending as it was causing problems on the CI.") <|
         remaining = GC_Example.perform_test 100000 (_->Nothing)
         remaining . should_equal 0

(additionally we can do it conditionally on the CI env var - we already have a few tests that are pending only on the CI - that has the advantage that any regressions are slightly more likely to be detected in some of the local runs we do from time to time)

@hubertp
Copy link
Collaborator Author

hubertp commented Jul 23, 2024

Unless I am greatly mistaken, the main method is only ran when manually running the GC_Example.enso script file.

To actually disable the test, you need to do the change elsewhere:

diff --git a/test/Base_Tests/src/Runtime/Managed_Resource_Spec.enso b/test/Base_Tests/src/Runtime/Managed_Resource_Spec.enso
--- a/test/Base_Tests/src/Runtime/Managed_Resource_Spec.enso	(revision a4259d31e0ea025aae4e50bbc8533f07ee536708)
+++ b/test/Base_Tests/src/Runtime/Managed_Resource_Spec.enso	(date 1721744396649)
@@ -58,7 +58,7 @@
         r_3 = Panic.recover Any <| Managed_Resource.bracket 42 (_-> Nothing) (_-> Panic.throw "action")
         r_3.catch . should_equal "action"
 
-    group_builder.specify "allocate lots of resources at once" <|
+    group_builder.specify "allocate lots of resources at once" pending=(if Environment.get "CI" . is_nothing . not then "GC Example is marked as pending as it was causing problems on the CI.") <|
         remaining = GC_Example.perform_test 100000 (_->Nothing)
         remaining . should_equal 0

(additionally we can do it conditionally on the CI env var - we already have a few tests that are pending only on the CI - that has the advantage that any regressions are slightly more likely to be detected in some of the local runs we do from time to time)

Ah, wasn't aware this was executed directly.

@hubertp hubertp added the CI: Ready to merge This PR is eligible for automatic merge label Jul 23, 2024
@mergify mergify bot merged commit 256a01a into develop Jul 23, 2024
37 checks passed
@mergify mergify bot deleted the wip/hubert/disable-gc-example branch July 23, 2024 17:26
jdunkerley pushed a commit that referenced this pull request Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants