This repository has been archived by the owner. It is now read-only.

Run fast testcases in Travis CI, remove obsolete testcase #760

Merged
merged 4 commits into from Oct 23, 2017

Conversation

@ollieparanoid
Member

ollieparanoid commented Oct 13, 2017

  • This adds test/testcases_fast.sh, which also gets executed by Travis CI now.
  • The ones which require cross-compilers or traffic-intense setup (cloning Alpine's aports repo) are disabled for now.
  • The apkindex_files cache has been removed in #345, this commit finally deletes the obsolete testcase, which has been failing since.

ollieparanoid added some commits Oct 13, 2017

Run fast testcases in Travis CI, remove obsolete testcase
(The apkindex_files cache has been removed in #345, this commit
finally deletes the obsolete testcase, which has been failing
since.)
@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Oct 13, 2017

Member

Interesting, two fail on Travis, but do not fail locally. I'll need some time to investigate and fix this (help welcome if anyone wants to).
Good news: test coverage is 52% right now (a lot more than I thought) 👍

Member

ollieparanoid commented Oct 13, 2017

Interesting, two fail on Travis, but do not fail locally. I'll need some time to investigate and fix this (help welcome if anyone wants to).
Good news: test coverage is 52% right now (a lot more than I thought) 👍

Try to fix both cases, that only fail on travis
test_challenge_apk:
This one failed, because we're accessing /etc/abuild.conf, which only
exists after initializing the build environment. It's a random dummy file
anyway, so I've replaced it with another file.

test_folder_size:
using the real folder size now, not quite sure where the additional bytes
for the directory come from. I guess that is a fixed value per folder?
Let's see what travis says.
@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Oct 16, 2017

Member

@ata2001 @craftyguy @drebrez @kskarthik @opendata26 @PabloCastellano @pavelmachek @PureTryOut @MartijnBraam @yuvadm @z3ntu @zhuowei:

It would be nice if you could take a look at this. The following function adds an offset to the folder size, although it should only return the size on disk as I understand it:

def folder_size(args, path):
"""
Run `du` to calculate the size of a folder (this is less code and
faster than doing the same task in pure Python).
:returns: folder size in bytes
"""
output = pmb.helpers.run.root(args, ["du", "--summarize",
"--apparent-size",
"--block-size=1",
path], return_stdout=True)
ret = int(output.split("\t")[0])
return ret

I assumed, that the offset is constant with 140 bytes, but that is not the case, as it fails on Travis:

    def test_get_folder_size(args, tmpdir):
        # Write five 2 KB files to tmpdir
        tmpdir = str(tmpdir)
        files = 5
        for i in range(files):
            pmb.helpers.run.user(args, ["dd", "if=/dev/zero", "of=" +
                                        tmpdir + "/" + str(i), "bs=1K",
                                        "count=2", "conv=notrunc"])
    
        # Check if the size is correct
        # 140 bytes seem to get added by the filesystem for the directory
        size = 2048 * files + 140
>       assert pmb.helpers.other.folder_size(args, tmpdir) == size
E       AssertionError: assert 14336 == 10380

(This is the only failing testcase in the PR.)

Member

ollieparanoid commented Oct 16, 2017

@ata2001 @craftyguy @drebrez @kskarthik @opendata26 @PabloCastellano @pavelmachek @PureTryOut @MartijnBraam @yuvadm @z3ntu @zhuowei:

It would be nice if you could take a look at this. The following function adds an offset to the folder size, although it should only return the size on disk as I understand it:

def folder_size(args, path):
"""
Run `du` to calculate the size of a folder (this is less code and
faster than doing the same task in pure Python).
:returns: folder size in bytes
"""
output = pmb.helpers.run.root(args, ["du", "--summarize",
"--apparent-size",
"--block-size=1",
path], return_stdout=True)
ret = int(output.split("\t")[0])
return ret

I assumed, that the offset is constant with 140 bytes, but that is not the case, as it fails on Travis:

    def test_get_folder_size(args, tmpdir):
        # Write five 2 KB files to tmpdir
        tmpdir = str(tmpdir)
        files = 5
        for i in range(files):
            pmb.helpers.run.user(args, ["dd", "if=/dev/zero", "of=" +
                                        tmpdir + "/" + str(i), "bs=1K",
                                        "count=2", "conv=notrunc"])
    
        # Check if the size is correct
        # 140 bytes seem to get added by the filesystem for the directory
        size = 2048 * files + 140
>       assert pmb.helpers.other.folder_size(args, tmpdir) == size
E       AssertionError: assert 14336 == 10380

(This is the only failing testcase in the PR.)

@zhuowei

This comment has been minimized.

Show comment
Hide comment
@zhuowei
Collaborator

zhuowei commented Oct 16, 2017

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Oct 16, 2017

Member

Yeah, I also think it makes it better, I've enabled it in the last commit (updated my previous comment to show the updated code snippet). But there are still some bytes getting added to the folder for some reason :\

Member

ollieparanoid commented Oct 16, 2017

Yeah, I also think it makes it better, I've enabled it in the last commit (updated my previous comment to show the updated code snippet). But there are still some bytes getting added to the folder for some reason :\

@drebrez

This comment has been minimized.

Show comment
Hide comment
@drebrez

drebrez Oct 17, 2017

Member

I did some investigation and found this:

$ find <folder> -type f -print0 | du --files0-from=- --summarize --apparent-size --block-size=1
2048    ./111
2048    ./333
2048    ./222
2048    ./444
2048    ./555
10240   total

what you think?

Member

drebrez commented Oct 17, 2017

I did some investigation and found this:

$ find <folder> -type f -print0 | du --files0-from=- --summarize --apparent-size --block-size=1
2048    ./111
2048    ./333
2048    ./222
2048    ./444
2048    ./555
10240   total

what you think?

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Oct 17, 2017

Member

@drebrez: Thanks for the idea! I've tried it, and I still get a different result on my PC compared to what you have.... I'm starting to doubt that it is possible to use du (which comes from "disk usage" apparently) to display the actual folder size, as if you would put it into an image file (although it says in the manpage that you can do just that with --apparent-size - it clearly does not work).

However, I think the roughly estimated size we get from du is good enough for our use case. So I've adjusted the testcase to be more tolerant with the output.

This PR is ready for review now.

Member

ollieparanoid commented Oct 17, 2017

@drebrez: Thanks for the idea! I've tried it, and I still get a different result on my PC compared to what you have.... I'm starting to doubt that it is possible to use du (which comes from "disk usage" apparently) to display the actual folder size, as if you would put it into an image file (although it says in the manpage that you can do just that with --apparent-size - it clearly does not work).

However, I think the roughly estimated size we get from du is good enough for our use case. So I've adjusted the testcase to be more tolerant with the output.

This PR is ready for review now.

@craftyguy

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Oct 17, 2017

Member

I haven't actually tried this, but instead of calling du (which may not be the same version in travis vs your system vs @drebrez's system), would something like os.walking a directory in python, calling os.path.getsize on each file in the directory's tree to add up their sizes lead to a deterministic total directory size across environments?

Member

craftyguy commented Oct 17, 2017

I haven't actually tried this, but instead of calling du (which may not be the same version in travis vs your system vs @drebrez's system), would something like os.walking a directory in python, calling os.path.getsize on each file in the directory's tree to add up their sizes lead to a deterministic total directory size across environments?

@craftyguy

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Oct 17, 2017

Member

lol, my suggestion might be what you are trying to change from. Sorry.

Member

craftyguy commented Oct 17, 2017

lol, my suggestion might be what you are trying to change from. Sorry.

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Oct 17, 2017

Member

We also did that at some point (I think @drebrez implemented it? or I've played with it) and it turned out to be really slow compared to du. But thanks for the idea!

Member

ollieparanoid commented Oct 17, 2017

We also did that at some point (I think @drebrez implemented it? or I've played with it) and it turned out to be really slow compared to du. But thanks for the idea!

@zhuowei

This looks good from a quick glance.

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Oct 23, 2017

Member

Thanks for having a look at it!

Member

ollieparanoid commented Oct 23, 2017

Thanks for having a look at it!

@ollieparanoid ollieparanoid merged commit ed94ab7 into master Oct 23, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ollieparanoid ollieparanoid deleted the feature/run-testsuite-in-travis branch Oct 23, 2017

PureTryOut added a commit that referenced this pull request Feb 21, 2018

Run fast testcases in Travis CI, remove obsolete testcase (#760)
* Removed obsolete apkindex_files cache testcase (the corresponding
function has been removed in #345 already).
* Fix test_challenge_apk: It failed on Travis, because we're accessing
/etc/abuild.conf, which only exists after initializing the build environment.
It's a random dummy file anyway, so I've replaced it with another file.
* Fix test_folder_size: accept a tolerance in the result
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.