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

Use Git to manage XDMoD test artifacts #143

Merged
merged 1 commit into from
Jun 7, 2017

Conversation

tyearke
Copy link
Contributor

@tyearke tyearke commented May 17, 2017

Description

This pull request moves responsibility for retrieval of xdmod-test-artifacts from Composer to the scripts that use it.

Motivation and Context

Copying/paraphrasing from an email exchange between @smgallo, @jpwhite4, and I:

Composer's design is built around pulling in runtime and development software packages in a consistent, well-considered manner, and the lock file's use of hashes for both individual dependencies and the contents of the lock file is meant to help guard against changes that haven't been tested. For example, if two pull requests both change the lock file and one gets merged, the other will be in conflict with the target branch due to the lock file's content hash being changed by both pull requests. As part of resolving the conflict, the outstanding pull request's lock file will need to be regenerated to include the changes from the other, which in so doing will (partially) validate the combined changes.

Having developers run commands that modify the lock file as part of the testing procedure leads to more work and more opportunities for accidentally pushing bad changes to the lock file. What this pull request does instead is have the scripts that use the test artifacts repo pull in the latest version of the repo with plain Git at run time. This means no updates to run manually and no files in the code repos to tweak and accidentally break when adding to the artifacts.

Tests Performed

Ran a test run of Travis on my fork and ran the unit tests manually on my local system.

Checklist:

  • My code follows the code style of this project as found in the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@tyearke tyearke added the qa label May 17, 2017
@tyearke tyearke added this to the v6.7.0 milestone May 17, 2017
@@ -14,5 +14,12 @@ if [ ! -x "$phpunit" ]; then
exit 127
fi

artifacts_dir="./xdmod-test-artifacts"
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this out to a separate setup script and then call it here. Also move the test artifacts directory somewhere else since it could (and will) be used by the integration_tests and regression_tests.

@tyearke
Copy link
Contributor Author

tyearke commented May 31, 2017

I've pushed a new commit that moves the artifact repo setup code from runtests.sh and the Travis scripts into a single new script. This script and the clone it creates/updates are now located in tests/artifacts instead of tests. When evaluating where to put them, I noticed that the locations of Open XDMoD test suites don't match the locations of those suites in the App Kernels and SUPReMM modules.

Project Unit Tests Integration Tests
Open XDMoD* tests integration_tests
SUPReMM tests/unit_tests tests/integration_tests
App Kernels tests/integration_tests

* = prepend open_xdmod/modules/xdmod/

My suggestion would be to (as part of a separate pull request) modify Open XDMoD's layout to match SUPReMM's layout. If that will be done, then tests/artifacts seems like a sensible location for the artifact-related assets to go in every repo.

I'll rebase the code to resolve conflicts if the changes I've made so far look good.

@tyearke tyearke changed the base branch from xdmod6.7 to xdmod7.0 June 6, 2017 15:15
@tyearke tyearke changed the base branch from xdmod7.0 to xdmod6.7 June 6, 2017 16:53
@tyearke tyearke changed the base branch from xdmod6.7 to xdmod7.0 June 6, 2017 16:53
@tyearke tyearke modified the milestones: v7.0.0, v6.7.0 Jun 6, 2017
@tyearke tyearke merged commit 97840f4 into ubccr:xdmod7.0 Jun 7, 2017
@tyearke tyearke deleted the qa/git-test-artifacts branch June 15, 2017 17:12
@plessbd plessbd added the qa / testing Updates/additions to tests label Mar 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
qa / testing Updates/additions to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants