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

TestDirectory reports number of failures and failed files #2899

Merged
merged 1 commit into from
Oct 9, 2018

Conversation

fingolfin
Copy link
Member

@fingolfin fingolfin commented Oct 5, 2018

With this, you may get output like this, with the
relevant change in the "total" line near the end.

gap> TestDirectory("misc");
Architecture: x86_64-apple-darwin15.6.0-default64-kv5

testing: misc/err.tst
       0 ms (0 ms GC) and 46.6KB allocated for err.tst
testing: misc/foo.tst
########> Diff in misc/foo.tst:3
# Input is:
1+1;
# Expected output:
3
# But found:
2
########
       0 ms (0 ms GC) and 6.62KB allocated for foo.tst
-----------------------------------
total         1 ms (0 ms GC) and 53.8KB allocated
              1 failures in 1 of 2 files

#I  Errors detected while testing

false
gap>

Resolves #2861

@fingolfin fingolfin added topic: tests issues or PRs related to tests release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Oct 5, 2018
@fingolfin fingolfin changed the title TestDirectory reports number of failures and failed fails TestDirectory reports number of failures and failed files Oct 5, 2018
@olexandr-konovalov
Copy link
Member

This will be useful @fingolfin - let's see how this will be seen in Travis.

@codecov
Copy link

codecov bot commented Oct 5, 2018

Codecov Report

Merging #2899 into master will decrease coverage by <.01%.
The diff coverage is 86.95%.

@@            Coverage Diff             @@
##           master    #2899      +/-   ##
==========================================
- Coverage   83.74%   83.74%   -0.01%     
==========================================
  Files         680      680              
  Lines      346265   346280      +15     
==========================================
+ Hits       289971   289979       +8     
- Misses      56294    56301       +7
Impacted Files Coverage Δ
lib/test.gi 65.05% <86.95%> (+0.73%) ⬆️
src/iostream.c 62.35% <0%> (-1.15%) ⬇️
hpcgap/lib/hpc/stdtasks.g 71.77% <0%> (-0.41%) ⬇️
lib/read4.g 100% <0%> (ø) ⬆️

@olexandr-konovalov
Copy link
Member

@fingolfin The problem with building manuals is not present in the master branch. Please rebase.

@fingolfin
Copy link
Member Author

What problem are you talking about? Travis and AppVeyor passed now. Also, what do you mean with "please rebase" -- this is already based on 94e61ab, the current master branch.

@fingolfin
Copy link
Member Author

(There simply was a stupid XML error in the GAPDoc, which is fixed now)

@olexandr-konovalov
Copy link
Member

I've been looking at the earlier test of this PR on Travis - that happened at https://travis-ci.org/gap-system/gap/builds/437551821. Indeed, fine now. Seen XML error in two separate PRs but not in the master branch, hence suggested to rebase...

@fingolfin
Copy link
Member Author

Ah I see: was confused because I saw your earlier comment pointing out broken XML; but after fixing that, the comment was gone?! Anyway, tests pass now.

Copy link
Member

@olexandr-konovalov olexandr-konovalov left a comment

Choose a reason for hiding this comment

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

Thanks @fingolfin - the code+docs look good, and I've fetched the PR and tested locally with both outcomes:

total     81576 ms (24411 ms GC) and 9.39GB allocated, 10 failures in 2 of 
220 files

and

total     78906 ms (23648 ms GC) and 9.34GB allocated, 0 failures in 0 of 
220 files
  • the total line becomes longer, maybe print the X failures in Y of Z files from a new line?
  • maybe 0 failures in 220 files rather than 0 failures in 0 of 220 files?

With this, you may get output like this, with the
relevant change in the "total" line near the end.

    gap> TestDirectory("misc");
    Architecture: x86_64-apple-darwin15.6.0-default64-kv5

    testing: misc/err.tst
           0 ms (0 ms GC) and 46.6KB allocated for err.tst
    testing: misc/foo.tst
    ########> Diff in misc/foo.tst:3
    # Input is:
    1+1;
    # Expected output:
    3
    # But found:
    2
    ########
           0 ms (0 ms GC) and 6.62KB allocated for foo.tst
    -----------------------------------
    total         1 ms (0 ms GC) and 53.8KB allocated
                  1 failures in 1 of 2 files

    #I  Errors detected while testing

    false
    gap>

Resolves gap-system#2861
@fingolfin
Copy link
Member Author

@alex-konovalov Done. One could next argue that 1 failures should be 1 failure, but I really don't want to spend more time on such minor details right now. If somebody is annoyed by that, I'll be happy to review their PR, though :-).

@ChrisJefferson ChrisJefferson merged commit 1cd18b7 into gap-system:master Oct 9, 2018
@fingolfin fingolfin deleted the mh/TestStats branch October 10, 2018 06:54
@olexandr-konovalov
Copy link
Member

@fingolfin should this be backported to stable-4.10?

@fingolfin
Copy link
Member Author

I don't see why.

@olexandr-konovalov
Copy link
Member

olexandr-konovalov commented Oct 16, 2018

Because it's useful and should not break things... Or, actually, because I don't want to maintain two different versions of scripts for jenkins which look up for the line with the word total at the end of the file with tail -5 | head -1 and tail -4 | head -1respectively :) (but I can).

@fingolfin
Copy link
Member Author

I don't find the first argument compelling, but the final one kinda is ;-)

@fingolfin
Copy link
Member Author

Backported via 9179000

@fingolfin fingolfin added backport-to-4.10-DONE release notes: added PRs introducing changes that have since been mentioned in the release notes and removed backport-to-4.10 release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Oct 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-4.10-DONE release notes: added PRs introducing changes that have since been mentioned in the release notes topic: tests issues or PRs related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants