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

Problem: nixpkgs is not pinned properly #235

Merged
merged 1 commit into from
Nov 3, 2020

Conversation

yihuang
Copy link
Collaborator

@yihuang yihuang commented Nov 2, 2020

nixpkgs is only pinned with environment variable, better to use niv to manage.

Solution:

  • use niv to manage nixpkgs and other dependencies
  • refactor nix expression a little bit

@tomtau
Copy link
Contributor

tomtau commented Nov 2, 2020

the ledger test fails @linfeng-crypto ?

==================================== ERRORS ====================================
4175
____________________ ERROR at setup of test_ledger_transfer ____________________
4176

4177
pytestconfig = <_pytest.config.Config object at 0x7fee73a14fd0>
4178
tmp_path_factory = TempPathFactory(_given_basetemp=None, _trace=<pluggy._tracing.TagTracerSub object at 0x7fee73838850>, _basetemp=PosixPath('/tmp/pytest-of-runner/pytest-1'))
4179

4180
    @pytest.fixture(scope="module")
4181
    def cluster(pytestconfig, tmp_path_factory):
4182
        "override cluster fixture for this test module"
4183
        ledger = get_ledger()
4184
        ledger.start()
4185
        try:
4186
>           yield from cluster_fixture(
4187
                Path(__file__).parent / "configs/ledger.yaml",
4188
                26800,
4189
                tmp_path_factory,
4190
                quiet=pytestconfig.getoption("supervisord-quiet"),
4191
            )
4192

4193
integration_tests/test_hardware_wallet.py:16: 

Solution:
- use niv to manage nixpkgs and other dependencies
- refactor nix expression a little bit
@yihuang
Copy link
Collaborator Author

yihuang commented Nov 2, 2020

the ledger test fails @linfeng-crypto ?

I think it's my fault, it's running with the chain-maind without ledger-zemu.

@codecov
Copy link

codecov bot commented Nov 2, 2020

Codecov Report

Merging #235 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #235   +/-   ##
=======================================
  Coverage   27.17%   27.17%           
=======================================
  Files          32       32           
  Lines        5773     5773           
=======================================
  Hits         1569     1569           
  Misses       4021     4021           
  Partials      183      183           
Flag Coverage Δ
integration_tests 27.17% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95dd975...ee980a4. Read the comment docs.

@@ -146,8 +146,8 @@ clean-docker-compose: localnet-stop
###############################################################################
# nix installation: https://nixos.org/download.html
nix-integration-test: check-network
nix-shell integration_tests/shell.nix --arg ci true --run "pytest -v -n 6 -m 'not ledger' --dist loadscope"
nix-shell integration_tests/shell.nix --arg ci true --run "pytest -v -m ledger"
nix-shell ./. -A ci-shell --run "pytest -v -n 7 -m 'not ledger' --dist loadscope"
Copy link
Contributor

@leejw51crypto leejw51crypto Nov 2, 2020

Choose a reason for hiding this comment

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

what's meaning of ./.?
if it means current folder, how about $PWD for readibility?

Copy link
Collaborator Author

@yihuang yihuang Nov 2, 2020

Choose a reason for hiding this comment

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

it's short for ./default.nix, I guess you can also use $PWD/.

nixpkgs-fmt
];
}
run: nix-shell ./. -A lint-shell --run "make lint-ci"
Copy link
Contributor

Choose a reason for hiding this comment

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

how about nix-shell $PWD?

Copy link
Contributor

@leejw51crypto leejw51crypto left a comment

Choose a reason for hiding this comment

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

lgtm

@tomtau tomtau merged commit 7510667 into crypto-org-chain:master Nov 3, 2020
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

Successfully merging this pull request may close these issues.

3 participants