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

Verify doc examples #48497

Closed
wants to merge 9 commits into from
Closed

Verify doc examples #48497

wants to merge 9 commits into from

Conversation

grahamc
Copy link
Member

@grahamc grahamc commented Oct 16, 2018

Motivation for this change

Test the function examples actually work.

Testing steps:

  1. enter doc/
  2. nix-shell
  3. make doc-tests

It outputs something like:

$ make doc-tests 
cd ./functions/library; python3 ./test.py
function-library-lib.attrset.attrByPath-example-value-exists: pass
function-library-lib.attrset.attrByPath-example-default-value: pass
function-library-lib.attrsets.recursiveUpdateUntil-example: pass
function-library-lib.attrsets.recursiveUpdate-example: pass
function-library-lib.attrsets.matchAttrs-example-matching-pattern-deep: pass
5 passed, 0 failed

For each example without a testable program, it prints:

Skipping example function-library-lib.attrsets.zipAttrs-example: no <programlisting role="input">

If a test fails, it outputs instructions on how to test only the failing examples:

function-library-lib.attrsets.recursiveUpdateUntil-example: pass
function-library-lib.attrsets.recursiveUpdate-example: pass
--------  Doc test failure  --------
ID: function-library-lib.attrsets.matchAttrs-example-matching-pattern-shallow
Executing:


      {
        actual = (let lib = import /home/grahamc/projects/nixpkgs/lib; in (
          
lib.attrsets.matchAttrs
  { cpu = {}; }
  { cpu = { bits = 64; }; }

        ));
        expect = (
          
  false

        );
      }
    

Expecting:

  false

Found:
True
----
--------  Doc test failure  --------
ID: function-library-lib.attrsets.matchAttrs-example-matching-pattern-deep
Executing:


      {
        actual = (let lib = import /home/grahamc/projects/nixpkgs/lib; in (
          
lib.attrsets.matchAttrs
  { host = { cpu = {}; }; }
  { host = { cpu = { bits = 64; }; }; }

        ));
        expect = (
          false
        );
      }
    

Expecting:
false
Found:
True
----
4 passed, 2 failed
Retry failing examples with:
./functions/library/test.py --lib-dir /home/grahamc/projects/nixpkgs/lib function-library-lib.attrsets.matchAttrs-example-matching-pattern-shallow function-library-lib.attrsets.matchAttrs-example-matching-pattern-deep

These tests will block the Nixpkgs manual from building as well.

The XML isn't super nice to write, but I think isn't too onerous?

Anyway, rendered it looks like this:

image

I also verified that python3 was already in the build closure.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • 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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

yield { "id": xml_id, "code": code.text, "expect": expect.text }


parser = argparse.ArgumentParser(description='Execute doc tests for the Nixpkgs manual..')
Copy link
Member

Choose a reason for hiding this comment

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

put the following code in a function, e.g. main() and add `if __name __ = "main": main()

doc_root = os.path.dirname(os.path.realpath(__file__))
all_doc_files = glob.glob("{}/*.xml".format(doc_root))

all_docs = [ET.parse(filename) for filename in all_doc_files]
Copy link
Member

Choose a reason for hiding this comment

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

generator expression

all_docs = [ET.parse(filename) for filename in all_doc_files]
all_examples = itertools.chain(*[find_examples(doc) for doc in all_docs])

if len(args.ids) > 1:
Copy link
Member

Choose a reason for hiding this comment

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

if args.ids:

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so, this is >1 not >0

Copy link
Member

Choose a reason for hiding this comment

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

... But should it be? seems like we drop a singleton list here? OK agreed with @FRidh then

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 this is a relic of when I didn't have proper argument parsing 😁 good catch.

doc/default.nix Outdated
@@ -6,7 +6,8 @@ in
pkgs.stdenv.mkDerivation {
name = "nixpkgs-manual";

buildInputs = with pkgs; [ pandoc libxml2 libxslt zip jing xmlformat ];
buildInputs = with pkgs; [ pandoc libxml2 libxslt zip jing xmlformat
python36 nix ];
Copy link
Member

Choose a reason for hiding this comment

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

python3 is sufficient. Note that soon I intend to make python37 the default, and this code will still work fine then.

Copy link
Member

@shlevy shlevy left a comment

Choose a reason for hiding this comment

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

Nice!

all_docs = [ET.parse(filename) for filename in all_doc_files]
all_examples = itertools.chain(*[find_examples(doc) for doc in all_docs])

if len(args.ids) > 1:
Copy link
Member

Choose a reason for hiding this comment

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

I don't think so, this is >1 not >0

all_docs = [ET.parse(filename) for filename in all_doc_files]
all_examples = itertools.chain(*[find_examples(doc) for doc in all_docs])

if len(args.ids) > 1:
Copy link
Member

Choose a reason for hiding this comment

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

... But should it be? seems like we drop a singleton list here? OK agreed with @FRidh then

failed = []

for example in filtered_examples:
full_expression = """
Copy link
Member

Choose a reason for hiding this comment

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

Maybe want f strings?

Using a single configuration file reduces the need to think about
how quoting works from Nix -> Bash -> Make -> xsltproc -> XSL

Additionally, it allows us to make additional interesting
customisations as needed.
@grahamc grahamc force-pushed the verify-doc-examples branch 2 times, most recently from 91869a6 to b216f74 Compare October 17, 2018 17:25
@mmahut
Copy link
Member

mmahut commented Aug 10, 2019

Are there any updates on this pull request, please?

@grahamc grahamc closed this Aug 10, 2019
@asymmetric
Copy link
Contributor

@grahamc has this been abandoned?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants