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

Please remove the test #26

Open
ip1981 opened this issue Nov 16, 2016 · 16 comments
Open

Please remove the test #26

ip1981 opened this issue Nov 16, 2016 · 16 comments

Comments

@ip1981
Copy link
Contributor

ip1981 commented Nov 16, 2016

The test is useless and when run automatically it likely fails for reason not related to the package.

See for example: NixOS/nixpkgs#20478

@paul-rouse
Copy link
Owner

Well, I put it there because I use it to test that building works with various combinations of packages (see the .travis.yml) Can't you simply arrange not to run it if you don't want it?

@ip1981
Copy link
Contributor Author

ip1981 commented Nov 16, 2016

Disabling it is not trivial in nixpkgs pinned to specific version. I'd use separate program/flag to run at travis.

@3noch
Copy link

3noch commented Nov 16, 2016

@ip1981 haskell.lib.dontCheck ?

@ip1981
Copy link
Contributor Author

ip1981 commented Nov 16, 2016

I'm looking for options. dontCheck will require a lot of levels of overrides. Meanwhile the test does not work for most of consumers.

@3noch
Copy link

3noch commented Nov 16, 2016

Alternatively you could try to get dontCheck into nixpkgs directly for this package. But I agree that's hacky.

@ip1981
Copy link
Contributor Author

ip1981 commented Nov 16, 2016

This works:

# cat modules/pkgs/mywatch/default.nix 
{ haskell, haskellPackages }:

let myHaskellPkgs = haskellPackages.override {
  overrides = self: super: {
    mysql        = haskell.lib.dontCheck super.mysql;
    mysql-simple = haskell.lib.dontCheck super.mysql-simple;
  };
};

in myHaskellPkgs.callPackage ./main.nix { }

But. The test is broken by design :)

@3noch
Copy link

3noch commented Nov 16, 2016

Yah I've had to do that with other packages. Not fun. Database packages seem to often require a lot of harness for tests to run.

@paul-rouse
Copy link
Owner

I think it is unhelpful to say that the "test is broken by design". It is the way it is to make my job easier, and over time it may well be extended to do more functional tests.

That said, I don't mind checking a flag or environment variable to stop the test trying to make a connection, but I would prefer to make the opt out explicit, leaving the default as it is.

@3noch
Copy link

3noch commented Nov 16, 2016

@paul-rouse The impedance mismatch is that the nixpkgs ecosystem enables tests for all packages by default. Certainly functional tests that rely on a working PostgreSQL instance, etc. are not easily supported by this design. I'm thinking it would just be better to have nixpkgs disable tests by default for this package.

@paul-rouse
Copy link
Owner

@3noch Yes, it does feel as if nixpkgs is really the right place - and of course stackage has a similar issue, but there is easy support for ignoring tests in that case. I'd still be happy to incorporate something if it makes life easier/cleaner for nixpkgs, though.

@3noch
Copy link

3noch commented Nov 16, 2016

@paul-rouse Anything that's not the "default" will be the same amount of work for nixpkgs because it will have to override.

@3noch
Copy link

3noch commented Nov 16, 2016

That said, some tests is better than no tests: if you had some way of disabling functional tests that might be the best.

@kquick
Copy link

kquick commented Nov 17, 2016

The current test would make a great example application, as an alternative to a test.

Having tests is great, but this is also going to break if there is an actual mysql server running that doesn't support the test user without a password. Maybe the test should be written to start an alternate mysql server at an unused port and with a pre-set configuration that matches the expectations of the test.

Thank you for maintaining this package (and mysql-simple), Paul!

@paul-rouse
Copy link
Owner

I am quite keen to keep this as a test, since it may well grow in the future to become more meaningful. I am also keen that a user who hasn't looked at the details, but just says stack test or cabal test, should get a pretty strong indication that they were unable to run the full test.

Now, a "strong indication" doesn't have to be a test failure in the technical sense: I would be happy with a big, fat, unmissable message explaining what needs to be done to run the test in full. Unfortunately, if this message is produced by the test itself, cabal hides it away in a log file unless the test does actually fail (stack does send it to the terminal in reasonable cases, and I am aware of the proposals for a similar default in haskell/cabal#1601 )

Any ideas?

@ghost
Copy link

ghost commented Apr 16, 2017

I am stumped on how to get output while keeping the tests passing...

Note: I generally have moved towards three test suites by default. Unit test. Basic integration tests. Extensive integration tests. Extensive integration tests get their own cabal file. The first two are in the main cabal file, with unit tests running by default and basic integration tests running as an opt-in by flag. You could force the basic integration test suite to run on PRs via Travis?

@ghost
Copy link

ghost commented Dec 6, 2017

https://hackage.haskell.org/package/gargoyle-postgresql-0.1/src/src/Gargoyle/PostgreSQL.hs - Obsidian recently used this to help Databrary run a database with minimal installation/hassle. I am not sure how much work it would be to port that to mysql, but if it were ported, that might provide some very convenient ways of writing tests that could be run anywhere with little ceremony.

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