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

Enable the ghcide test plugin in HLS test suites #2243

Merged
merged 11 commits into from
Oct 4, 2021
Merged

Conversation

pepeiborra
Copy link
Collaborator

@pepeiborra pepeiborra commented Sep 26, 2021

The ghcide test plugin installs a CustomMethod handler that allows to interact with the build queue and do things like waiting for the queue to be empty. It's installed by the ghcide main driver but not by the HLS test driver.

This PR refactors things so that it will be installed automatically by the defaultMain driver.

@pepeiborra pepeiborra added the merge me Label to trigger pull request merge label Sep 26, 2021
@pepeiborra pepeiborra force-pushed the ghcide-test-plugin branch 4 times, most recently from fe1f476 to c37c2a2 Compare September 27, 2021 23:50
@jneira
Copy link
Member

jneira commented Sep 28, 2021

Not sure what dark spell made #2060 pass ci (:stuck_out_tongue_closed_eyes:) but it is failing since then more frequently, for tactics and rename plugin.
I tested manually in my machine renaming works fine with master after #2060 but i've not run tests yet.

@jneira
Copy link
Member

jneira commented Sep 28, 2021

The tests run all in parallel on the same workspace, and most of the modules have the same module name Main. Result: parallel writes to the same .hie files in the local tmp folder that lead to IO errors in Windows

thanks, i was about to make optional the rename test suite for windows and ghc-9.0.1

@jneira
Copy link
Member

jneira commented Sep 29, 2021

/rerun-workflow Testing

@jneira
Copy link
Member

jneira commented Sep 29, 2021

eval plugin tests failed for linux int two consecutive runs

@pepeiborra
Copy link
Collaborator Author

My recent commit broke it - still working on it

@pepeiborra pepeiborra force-pushed the ghcide-test-plugin branch 3 times, most recently from 9dc743e to 44bddfa Compare September 30, 2021 22:22
@@ -30,6 +30,8 @@ jobs:
test:
needs: pre_job
runs-on: ${{ matrix.os }}
env:
TASTY_NUM_THREADS: 1
Copy link
Member

@jneira jneira Oct 1, 2021

Choose a reason for hiding this comment

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

this has a different effect from pass -j1 to the test executable?

Copy link
Member

Choose a reason for hiding this comment

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

it seems the env var is equivalent to the arg, if docs are correct: https://github.com/UnkindPartition/tasty#runtime

Copy link
Collaborator Author

@pepeiborra pepeiborra Oct 1, 2021

Choose a reason for hiding this comment

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

It is equivalent, but I don't think it worked. Did I put it in the right place?

Copy link
Member

Choose a reason for hiding this comment

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

but we are already passing -j1 to tasty via --test-options in the workflow, do you suspect the cli arg could have a bug?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh you are right, I missed that -j1 is already there

Copy link
Member

Choose a reason for hiding this comment

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

it is a shot in the dark but i am trying with -j1 directly for cabal

Instead, send all ghcide output through the logger and keep stderr open for fatals
@pepeiborra
Copy link
Collaborator Author

The problem with the tactics test suite was also Main modules. Fixed it in my last commit, this PR should be getting merged very soon.

@mergify mergify bot merged commit edf7be5 into master Oct 4, 2021
@jneira
Copy link
Member

jneira commented Oct 4, 2021

The problem with the tactics test suite was also Main modules. Fixed it in my last commit, this PR should be getting merged very soon.

many many thanks, in my last try almost all tactic tests failed with session timeouts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants