Skip to content

Force Python 3 test failures to fail the build#81

Merged
blag merged 1 commit intomasterfrom
turn-on-python3-checks
Feb 1, 2020
Merged

Force Python 3 test failures to fail the build#81
blag merged 1 commit intomasterfrom
turn-on-python3-checks

Conversation

@blag
Copy link
Contributor

@blag blag commented Feb 1, 2020

No description provided.

@punkrokk
Copy link
Contributor

punkrokk commented Feb 1, 2020

Am I wrong or should this really require a script to create a PR in each pack repo to resolve broken py3 tests? Is it reasonable to expect the next person that happens to do a PR to have to fix the tests?

@nmaludy
Copy link
Contributor

nmaludy commented Feb 1, 2020

Yeah we really need tooling to help with that.

In Puppet land we use either modulesync or PDK. Basically both tools have a 'template" repo they base off of, then when you run the sync it clones your main repo and syncs with the template.

Modulesync, I believe, also had the ability to make changes over a big list of repos. Think of this like cookiecutter on steroids.

https://github.com/voxpupuli/modulesync
https://github.com/voxpupuli/modulesync_config

https://github.com/puppetlabs/pdk
https://github.com/puppetlabs/pdk-templates

I'm sure we could use these tools. Not sure if writing something ourselves is worthwhile?

@blag
Copy link
Contributor Author

blag commented Feb 1, 2020

You're not wrong, but I'm the one working on fixing all packs that don't work on Python 3. It's a long story, but I was going to merge this in before our weekly pack CI runs (which test every Python file in the pack, and not just changed Python files like the PR tests do) this weekend, so I could see packs actually fail. And I plan on reverting this on Monday.

The way we have it setup now, the Python 3 PRs can pass, but once you merge into the pack master branch, that CI run fails. 😡 See my recent AWS pack PR (test results)for an example of that (test results after merge to master).

@blag
Copy link
Contributor Author

blag commented Feb 1, 2020

Tested this with the AWS pack, results here:
https://circleci.com/gh/StackStorm-Exchange/stackstorm-aws/530

I'm going to merge this in for the weekend pack CI runs, and then revert it on Monday.

@blag
Copy link
Contributor Author

blag commented Feb 1, 2020

I should also add that for pack PRs this weekend, the CircleCI builds will only fail (due to this change) if they touch a module that is not Python 3 compliant and do not fix the module themselves (because CI runs for PRs only check files that are changed in the PR itself, not all files).

Since most of our pack PRs happen on weekdays, and our CI for PRs only checks changed files, I think the impact on other users will be minimal this weekend.

@punkrokk
Copy link
Contributor

punkrokk commented Feb 1, 2020 via email

Copy link
Contributor

@nmaludy nmaludy left a comment

Choose a reason for hiding this comment

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

LGTM

@punkrokk punkrokk self-requested a review February 1, 2020 05:20
Copy link
Contributor

@punkrokk punkrokk left a comment

Choose a reason for hiding this comment

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

Agreed - LGTM

@blag blag merged commit d6f38ae into master Feb 1, 2020
@blag blag deleted the turn-on-python3-checks branch February 1, 2020 05:50
@blag blag mentioned this pull request Feb 2, 2020
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.

3 participants