Skip to content

Cleanup func-test suite #3828

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 6 commits into from
Oct 9, 2023
Merged

Conversation

fendor
Copy link
Collaborator

@fendor fendor commented Oct 5, 2023

This test-suite is vastly legacy from Haskell IDE Engine. It was originally written before we had a plugin-based architecture, where each plugin tests its feature in isolation.
Over time, this test-suite bitrotted, where a lot of testcases were either redundant or out-of-date and consequentially disabled.
Further, make sure we only have tests here that need to be integration tests.

We clean up the test-suite, delete old tests and remove unused testdata.

Before we merge:

  • Move tests for qualified imports to the plugin the feature is defined in
  • Write a README for guidance when to add a test to func-test.

closes #3365

@fendor fendor force-pushed the enhance/overhaul-func-test branch from 747ace8 to 7b5b7ac Compare October 5, 2023 18:03
@fendor fendor requested a review from santiweight as a code owner October 5, 2023 19:13
Comment on lines +6 to +7
/hls-test-utils @fendor
/test @fendor
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I noticed that hls-test-utils has no codeowner, I am volunteering to take over the ownership.
Additionally, added myself as codeowner for func-test. Mostly, to make sure my somewhat arbitrary guidance in test/README.md cannot be further violated without me noticing.

@fendor fendor force-pushed the enhance/overhaul-func-test branch from 4dd9981 to 5ba5c7a Compare October 6, 2023 08:08
Copy link
Collaborator

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

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

FunctionalBadProject and HieBios also looks like they could be in ghcide?

@fendor
Copy link
Collaborator Author

fendor commented Oct 6, 2023

Yeah, that's true.

@fendor fendor force-pushed the enhance/overhaul-func-test branch 2 times, most recently from 6723923 to 3bd5190 Compare October 6, 2023 09:40
@fendor
Copy link
Collaborator Author

fendor commented Oct 6, 2023

I think, the only tests that should really be in func-test are the formatter provider tests, and the config change tests.

The rest could live somewhere else...

@fendor fendor force-pushed the enhance/overhaul-func-test branch from 3bd5190 to 7ae93da Compare October 6, 2023 11:20
@fendor
Copy link
Collaborator Author

fendor commented Oct 7, 2023

For some reason, the tests get slower if I move them to ghcide. Until I got to the bottom of this, I won't move the tests, and leave them here.
Merge as-is to speed up CI a little bit.

fendor added 5 commits October 7, 2023 11:44
This test-suite is vastly legacy from Haskell IDE Engine.
It was originally written before we had a plugin-based architecture
where each plugin tests its feature in isolation.
Over time, this test-suite bitrotted, where a lot of testcases were
either redundant or out-of-date and consequentially disabled.

We clean up the test-suite, delete old tests and remove unused testdata.
Provides guidance when to add a test to `func-test` and some historical
context.
@fendor fendor force-pushed the enhance/overhaul-func-test branch from 7ae93da to cb2b380 Compare October 7, 2023 09:44
@fendor fendor added the merge me Label to trigger pull request merge label Oct 7, 2023
@mergify mergify bot merged commit 168976d into haskell:master Oct 9, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Review ignored tests in func-test
2 participants