Skip to content
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

Fix testdata hie.yaml generation #517

Closed
jneira opened this issue Oct 19, 2020 · 6 comments
Closed

Fix testdata hie.yaml generation #517

jneira opened this issue Oct 19, 2020 · 6 comments
Labels
CI Continuous integration

Comments

@jneira
Copy link
Member

jneira commented Oct 19, 2020

But now I'm stumped as to how the tests are working in the first place even

@bubba @fendor @isovector me too, analyzing the test code and data:

  • Most tests are being ignored so although there is no hie.yaml, they pass
  • Working tests (eval, tactic, etc) have a hie.yaml in its subdirectory, despite they were intended to be ignored with

# ignore hie.yaml's for testdata
test/testdata/**/hie.yaml

I suppose they were added with git add --force. 😺

  • Moreover the hie.yaml generated by setupBuildToolFiles will not work anymore, as they should include modules explicitly

So, i would ignore hie.yaml's but:

  • setupBuildToolFiles has to include auto all modules in the directory where it is about to be created (not sure if it would be feasible)
  • specific tests should call a version of setupDirectFilesIn with a new parameter with the modules to add in the direct cradle
    • and remove the generic setupBuildToolFiles, or add the modules for each test suite there
@isovector
Copy link
Collaborator

Yeah, I force added my hie.yaml after a few hours of debugging why CI was failing but everything worked fine locally.

@jneira
Copy link
Member Author

jneira commented Oct 19, 2020

@isovector hlint plugin tests was failing for the same reason but then i've remembered your issue/pr relatively early.
Finally i've added the hie.yaml too and i hope we can drop them at once.
Not sure if hie.yaml schema will change but we will have one point to change it.

peterwicksstringfield added a commit to peterwicksstringfield/haskell-language-server that referenced this issue Dec 2, 2020
This is a cludge. We should be auto-generating these files. See haskell#517. But for
now we can get the tests running, then fix that part later.
@peterwicksstringfield
Copy link
Contributor

We just deleted the "real" cradles (in #741 via #1230) that we use for actually developing hls (and good riddance, if hls can sort-of just-work without users making weird helper files that's great)

... so it seems like a good time to revive this issue.

As konn mentions (#741 (comment)) I think we need the ability to handwrite cradles for testing. So long as cradles exist we will want to test what happens when they are weird / malformed / handwritten. In particular, our testdata currently includes "fake" cradles like this:

cradle:
  direct:
    arguments:
      - "NeedsPragmas"

And:

cradle:
  direct:
    arguments:
      - "-XCPP"
      - "-DFLAG"
      - "ApplyRefact3"
      - "ApplyRefact2"

There's no way we are auto-generating that.

Do we want to leave the "fake" cradles in the testdata where they are and close this issue ... or do something else?

If we are worried about the "real" hls seeing the "fake" cradles and causing problems while we are developing, then one option would be to rename the "fake" cradles from "testdata/../hie.yaml" to "testdata/../test-hie.yaml", then just before the tests run, we copy "test-hie.yaml" back to "hie.yaml", then after the tests run we can delete all the "hie.yaml" again. (I partially implemented this and it seems like it works just fine.)

@jneira
Copy link
Member Author

jneira commented Jan 20, 2021

There's no way we are auto-generating that.

Do we want to leave the "fake" cradles in the testdata where they are and close this issue ... or do something else?

Well, we can always write code parameterized enough to generate the files with something like:

generateTestCradle :: Filepath -> [String]
generateTestCradle dir args = ...

and make it generate a direct cradle in dir. Then we can add auxiliary functions to generate common direct cradles. Test code will have to call that function with appropiate arguments for each case. When all tests use that function we could remove them all from vcs again.

If we are worried about the "real" hls seeing the "fake" cradles and causing problems while we are developing

Well, in my case i like to open the testdata dir and test manually in the editor before or while i am writing the test, it is not real code so they did not cause problems for me, do they for you?

@jneira
Copy link
Member Author

jneira commented Feb 24, 2021

Not sure if at this point this continue being relevant

@jneira jneira added the status: needs info Not actionable, because there's missing information label Feb 24, 2021
@jneira
Copy link
Member Author

jneira commented Mar 21, 2021

It is not 🙂

@jneira jneira closed this as completed Mar 21, 2021
@jneira jneira removed the status: needs info Not actionable, because there's missing information label Mar 21, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
CI Continuous integration
Projects
None yet
Development

No branches or pull requests

3 participants