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

Remove duplicated tests between dhall-lang and dhall-haskell #766

Closed
jneira opened this issue Dec 28, 2018 · 10 comments
Closed

Remove duplicated tests between dhall-lang and dhall-haskell #766

jneira opened this issue Dec 28, 2018 · 10 comments

Comments

@jneira
Copy link
Collaborator

jneira commented Dec 28, 2018

Wouldn't be desirable avoid the duplication between spec tests in this repo and the dhall-lang ones?
Modifications could be:

  • Remove spec tests from dhall-haskell
  • Include dhall-lang as submodule of dhall-haskell (like dhall-clj)
  • Read spec tests cases from there and the implementation specific ones from the actual location
  • Set up some way to skip spec tests explicitly in case we want to ship a dhall-haskell version not 100% compliant
@Gabriella439
Copy link
Collaborator

@jneira: The main limitation here is that I haven't figured out how to get Hydra to work with git submodules. If somebody can get it working I'd accept the change

@jneira
Copy link
Collaborator Author

jneira commented Dec 30, 2018

Mmm another way could be parametrize the spec tests location and set by default to ../dhall-lang/tests, but it implies make two steps to run the tests. If Hydra is able to checkout other repos, it should work.
That is my setup to run dhall-eta tests: https://github.com/eta-lang/dhall-eta/blob/master/.circleci/config.yml#L47

We should document the steps and print an error in case the tests suite is executed without spec tests in the selected location.

@ocharles
Copy link
Member

ocharles commented Dec 30, 2018 via email

@ocharles
Copy link
Member

ocharles commented Jan 3, 2019

It turns out there are no patches needed. Here's our hydra.nix Nixpkgs overlay:

self: super:
{ hydra =
    super.hydra.overrideAttrs
      ( _:
        { src =
            self.fetchFromGitHub
              { owner = "NixOS";
                repo = "hydra";
                rev = "63a294d4caa1124dd1c7d08c06ccef4113154bc8";
                sha256 = "0xw6kph7zxlq2inb4aglp8g0hinnyz8z4x2dmkjnc2lhkhsdaq1j";
              };

          patches =
            [ # Fix for https://github.com/NixOS/nix/issues/1888
              ./hydra/unrestrict-hydra.diff

              ./hydra/job-name.patch

              ./hydra/hydra-auth-file.patch

              ./hydra/faster-clone.patch
            ];

          doCheck =
            false;
        }
      );
}

None of those patches change submodule behaviour.

@f-f
Copy link
Member

f-f commented Jan 15, 2019

@ocharles @Gabriel439 any further ideas on this? Could it be this issue?

@ocharles
Copy link
Member

ocharles commented Jan 15, 2019

I would like to know if using the above version of Hydra works. Without knowing the version that doesn't work it's hard to give more advice.

@Gabriella439
Copy link
Collaborator

@ocharles: Yeah, I need to pin Dhall CI to a specific revision of Nixpkgs. The Nixpkgs revision I've mostly recently deployed from is:

... which corresponds to this revision of Hydra:

... which is a couple of months older than the revision you are using:

Here is the difference between the two revision:

NixOS/hydra@4dca8fe...63a294d

From a brief scan of that, it looks like this submodule-related commit might fix things: NixOS/hydra@8bffbb7

I'll try to see if deploying that newer revision of Hydra fixes things

@Gabriella439
Copy link
Collaborator

Looks like submodules are back on the menu: #787

@jneira
Copy link
Collaborator Author

jneira commented Jan 19, 2019

Hi, @Gabriel439 test code should be updated to read the tests from the new locations. Should we open a new one about?

@Gabriella439
Copy link
Collaborator

@jneira: I did update the test code to read the test files from the new location

dhess pushed a commit to hackworthltd/hacknix that referenced this issue Jan 21, 2021
It appears the git submodule issues have been resolved upstream; see:

dhall-lang/dhall-haskell#766

We also probably don't need the hydra-no-restrict patch anymore, so
long as we add this to the NixOS Hydra module config:

https://github.com/cleverca22/nixos-configs/blob/7322b81f4fa7271c015014ed44595030f700b862/nas-hydra.nix#L32-L34

See NixOS/nix#1888 (comment)

We keep the secure GitHub token-handling patch, and add a patch which
enables verbose mode when `hydra-eval-jobs` runs, for better logging
and some visibility into what Hydra is up to during evaluations.
dhess pushed a commit to hackworthltd/hacknix that referenced this issue Jan 21, 2021
It appears the git submodule issues have been resolved upstream; see:

dhall-lang/dhall-haskell#766

We also probably don't need the hydra-no-restrict patch anymore, so
long as we add this to the NixOS Hydra module config:

https://github.com/cleverca22/nixos-configs/blob/7322b81f4fa7271c015014ed44595030f700b862/nas-hydra.nix#L32-L34

See NixOS/nix#1888 (comment)

We keep the secure GitHub token-handling patch, and add a patch which
enables verbose mode when `hydra-eval-jobs` runs, for better logging
and some visibility into what Hydra is up to during evaluations.
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

No branches or pull requests

4 participants