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

(PUP-3974) test source attribute, all checksums #3674

Conversation

er0ck
Copy link
Contributor

@er0ck er0ck commented Mar 4, 2015

Prior to this change the source_attributes test did not cover the puppet
apply cases for all checksums. This change adds these tests.
This change also removes the sleep(1) in favor of a loop which checks to
ensure we're ready to move on, saving up to a second.

Prior to this change the source_attributes test did not cover the puppet
apply cases for all checksums.  This change adds these tests.
This change also removes the sleep(1) in favor of a loop which checks to
ensure we're ready to move on, saving up to a second.
on agent, "mkdir -p #{File.dirname(source)}"
create_remote_file agent, source, 'Yay, this is the local file.'

step "Using a local file path (#{checksum})"
Copy link
Contributor

Choose a reason for hiding this comment

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

it was surprising to me that checksum included the trailing comma, which led to unintuitive log messages:

Using a local file path (checksum => ctime,)

Can we move the comma to the manifest where the checksum is used, or just remove entirely (since no properties/parameters come after checksum in the manifests below)?

@joshcooper
Copy link
Contributor

This test passed for rhel7 and win2012r2-rubyx64, but it takes ~ 10 mins on Windows because it runs puppet apply twice for each checksum type, where each run takes ~ 16 seconds.

be rake ci:test:aio CONFIG=config/nodes/win2012r2-rubyx64.yaml  SHA=215e36f962dfd7080947d78a1a1340b0aadcfa62 TESTS=tests/resource/file/source_attribute.rb
...
seo2bu30c2m5h80.delivery.puppetlabs.net (agent-2012r2-x86_64-rubyx64) 00:55:22$ cmd.exe /c puppet apply --modulepath=C:/cygwin64/tmp/local_source_file_test.XP46nu -e "file { 'C:/cygwin64/tmp/local_source_file_test.XP46nu/targetmtime': source => 'puppet:///modules/source_mod/source', ensure => present, checksum => mtime, }"
...
seo2bu30c2m5h80.delivery.puppetlabs.net (agent-2012r2-x86_64-rubyx64) executed in 16.06 seconds
...
       Total Suite Time: 613.05 seconds
      Average Test Time: 613.05 seconds
              Attempted: 1
                 Passed: 1
                 Failed: 0
                Errored: 0
                Skipped: 0
                Pending: 0
                  Total: 1

Can we rework this test to create a single manifest with a number of file resources, run puppet apply once to create the initial files, then run puppet apply again and verify no changes are necessary?

We have the same issue with the symbolic file mode test and I'd prefer not to add another 10mins to the test suite time.

@kylog kylog added the PL label Mar 4, 2015
Eric Thompson added 2 commits March 10, 2015 13:59
previous commit placed two puppet applies inside a checksum loop. the
test took 661s to run on windows. refactor the test to do single puppet
applies for each segment. test now takes ~100s.
add tests to ensure updates are not made when changes are made outside
of a source files <checksum>lite purview
@er0ck
Copy link
Contributor Author

er0ck commented Mar 10, 2015

good comments @joshcooper
windows runtime down to ~100 seconds \o/
added the lite tests beyond 512 bytes as well. nice

MikaelSmith pushed a commit that referenced this pull request Mar 17, 2015
…ource_attribute-all_checksums

(PUP-3974) test source attribute, all checksums
@MikaelSmith MikaelSmith merged commit 190fdaf into puppetlabs:master Mar 17, 2015
@er0ck er0ck deleted the acceptance/master/PUP-3974-test_source_attribute-all_checksums branch February 3, 2016 19:05
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.

None yet

4 participants