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

Update the hooks used in the documentation and tests #943

Merged
merged 2 commits into from
Apr 2, 2022

Conversation

mwouts
Copy link
Owner

@mwouts mwouts commented Apr 2, 2022

The CI is failing on the test test_pre_commit_hook_sync_black_nbstripout, see also #940 and #942

I cannot reproduce that error locally, so I am trying to update the hooks used by this test.

The details for the error are below:

    @skip_pre_commit_tests_on_windows
    @skip_pre_commit_tests_when_jupytext_folder_is_not_a_git_repo
    def test_pre_commit_hook_sync_black_nbstripout(
        tmpdir,
        cwd_tmpdir,
        tmp_repo,
        jupytext_repo_root,
        jupytext_repo_rev,
        notebook_with_outputs,
    ):
        """Here we sync the ipynb notebook with a py:percent file and also apply black and nbstripout."""
        pre_commit_config_yaml = f"""
    repos:
    - repo: {jupytext_repo_root}
      rev: {jupytext_repo_rev}
      hooks:
      - id: jupytext
        args: [--sync, --pipe, black]
        additional_dependencies:
        - black==20.8b1  # Matches hook
    
    - repo: https://github.com/psf/black
      rev: 20.8b1
      hooks:
      - id: black
    
    - repo: https://github.com/kynan/nbstripout
      rev: 0.3.9
      hooks:
      - id: nbstripout
    """
        tmpdir.join(".pre-commit-config.yaml").write(pre_commit_config_yaml)
    
        tmp_repo.git.add(".pre-commit-config.yaml")
        pre_commit(["install", "--install-hooks", "-f"])
    
        tmpdir.join(".jupytext.toml").write('formats = "ipynb,py:percent"')
        tmp_repo.git.add(".jupytext.toml")
        tmp_repo.index.commit("pair notebooks")
    
        # write a test notebook
        write(notebook_with_outputs, "test.ipynb")
    
        # try to commit it, should fail because
        # 1. the py version hasn't been added
        # 2. the first cell is '1+1' which is not black compliant
        # 3. the notebook has outputs
        tmp_repo.git.add("test.ipynb")
        with pytest.raises(
            HookExecutionError,
            match="files were modified by this hook",
        ):
            tmp_repo.index.commit("failing")
    
        # Add the two files
        tmp_repo.git.add("test.ipynb")
>       tmp_repo.git.add("test.py")
 >           raise GitCommandError(redacted_command, status, stderr_value, stdout_value)
E           git.exc.GitCommandError: Cmd('git') failed due to: exit code(128)
E             cmdline: git add test.py
E             stderr: 'fatal: pathspec 'test.py' did not match any files'

@codecov
Copy link

codecov bot commented Apr 2, 2022

Codecov Report

Merging #943 (a3cb07b) into main (adff5a2) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #943   +/-   ##
=======================================
  Coverage   99.10%   99.10%           
=======================================
  Files         117      117           
  Lines       10869    10869           
=======================================
  Hits        10772    10772           
  Misses         97       97           
Impacted Files Coverage Δ
tests/test_pre_commit_2_sync_nbstripout.py 100.00% <ø> (ø)
tests/test_pre_commit_3_sync_black_nbstripout.py 100.00% <ø> (ø)
tests/test_pre_commit_5_reformat_markdown.py 100.00% <ø> (ø)

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 adff5a2...a3cb07b. Read the comment docs.

@mwouts mwouts merged commit 3ad96e2 into main Apr 2, 2022
@mwouts mwouts deleted the update_pre_commit_hooks branch April 2, 2022 11:01
@mwouts mwouts mentioned this pull request Apr 2, 2022
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.

1 participant