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.

Sign up for GitHub

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

Jump to bottom

nixos/tests: fix pgjwt test #29927

Merged
merged 4 commits into from Oct 4, 2017
Merged

nixos/tests: fix pgjwt test #29927

merged 4 commits into from Oct 4, 2017

Conversation

WilliButz
Copy link
Member

Things done
  • updated pgjwt to the most recent revision and adjusted the version numbering
  • updated perlPackages.TAPParserSourceHandlerpgTAP from 3.30 to 3.33
  • added the pgtap extension for postgresql
  • rewrote the pgjwt test so that is now uses the test from the pgjwt repo which is much more extensive than our test was before
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

joachifm
joachifm reviewed Oct 1, 2017
View reviewed changes
nixos/tests/pgjwt.nix Outdated
select * from verify('eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWV9.TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ', 'secret');
'';
test = (pkgs.fetchurl {
url = https://raw.githubusercontent.com/michelp/pgjwt/master/test.sql;
Copy link
Contributor

Choose a reason for hiding this comment

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

Refering to a specific commit seems more robust

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated it @joachifm. It now uses the same revision pgjwt has, so that the test- and package versions should always be compatible. I also added a comment to the pgjwt/default.nix to update the hash in the test file with each revision change.

Copy link
Contributor

Choose a reason for hiding this comment

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

I take it that the test file comes from the same source as the package itself. In that case you can exploit the fact that fetchFromGitHub is based on fetchzip which produces an unpacked source archive, allowing you to refer to test file as ${pgjwt.src}/test.sql. A more generic solution is to include test.sql in the package output and refer to it that way (e.g., ${pgjwt}/share/pgjwt/test.sql) or even as a separate output if it is very large.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@WilliButz WilliButz force-pushed the fix-pgjwt-test branch 3 times, most recently from 3477e57 to e4ea265 Compare October 1, 2017 17:13
@WilliButz
pgjwt: 0.0.1 -> unstable-2017-04-24
d387bc8 
- updated to the latest revision
- fixed version format as there are no releases yet
@WilliButz
perlPackages/TAPParserSourceHandlerpgTAP: 3.30 -> 3.33
d096041 
- update and refactor a bit
@WilliButz
pgtap: init at version 0.97.0
2ebc40f
@WilliButz
nixos/tests: rewrite pgjwt test
7d09fc6 
- now using the test contained in the pgjwt source repo
- also compatible with the new `superUser` option of the
  `postgresql` service
@joachifm
Copy link
Contributor

joachifm commented Oct 3, 2017

cc @spinus

@joachifm joachifm merged commit 0625110 into NixOS:master Oct 4, 2017
@joachifm
Copy link
Contributor

joachifm commented Oct 4, 2017

Thank you

@WilliButz
Copy link
Member Author

@joachifm I was just cleaning up the test some more ^^
It's this PR if you don't mind taking a look at it as well :)

@spinus
Copy link
Member

spinus commented Oct 4, 2017

@WilliButz thank you! great stuff.

@vcunat
Copy link
Member

vcunat commented Oct 5, 2017

Typo in hash broke evaluation; fixed in 583b8c1.

@vcunat
Copy link
Member

vcunat commented Oct 5, 2017

Background: some older nix versions disregarded the few extra bits at the beginning, but newer versions throw an error.

@WilliButz
Copy link
Member Author

I'm on Nix 1.11.15, I wonder how it did not throw an error for me during evaluation %)

@vcunat
Copy link
Member

vcunat commented Oct 5, 2017

Aha, 1.11.x doesn't have this :-/ See NixOS/nix#1121

@WilliButz
Copy link
Member Author

Ah alright, good to know that 👍

@vcunat
Copy link
Member

vcunat commented Oct 5, 2017

The fork-off between master and 1.11.x was in January 2016 – I keep forgetting that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joachifm joachifm joachifm left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@WilliButz @joachifm @spinus @vcunat