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

[DX] Exclude .tests.info files from requiring a "type =" line. #1294

Closed
klonos opened this issue Oct 3, 2015 · 11 comments
Closed

[DX] Exclude .tests.info files from requiring a "type =" line. #1294

klonos opened this issue Oct 3, 2015 · 11 comments

Comments

@klonos
Copy link
Member

klonos commented Oct 3, 2015

These are not modules or themes or layouts. The "right" way to do it would be to perhaps require a type = test line, but I say we make this optional and instead rely on the fact that these live within /test subdirectories.

This came up in backdrop-contrib/honeypot#6 and elsewhere where modules would not be installed because we check and validate .info files in every subdir inside a project. Lets ease DX and improve UX.

Steps to reproduce:
1)
2)
3)

Backdrop version 1.2.2


PR: backdrop/backdrop#2491

@quicksketch
Copy link
Member

Rather than ignoring test directories, I'd recommend just stopping at the first .info file found, as suggested in backdrop-contrib/honeypot#6 (comment). It will accomplish the same thing in hopefully a slightly more robust approach.

@quicksketch
Copy link
Member

Oh, that other issue is in Honeypot's queue, not core. Reopening to fix this here.

@pifbits
Copy link

pifbits commented Mar 24, 2016

I'm still having the issue honeypot. Manually install via cmd/file manager works tho. it's also happening with mimemail, civicrm, and a few others. I'm just testing config modules for now. I'm not even at the point of dropping tables and re-importing a db-copy from the d7 production site yet.

@jenlampton jenlampton changed the title [DX] Exclude .info files within /test directories from requiring a "type =" line. [DX] Exclude .tests.info files within /test directories from requiring a "type =" line. Sep 7, 2017
@jenlampton jenlampton changed the title [DX] Exclude .tests.info files within /test directories from requiring a "type =" line. [DX] Exclude .tests.info files from requiring a "type =" line. Sep 7, 2017
@jenlampton
Copy link
Member

jenlampton commented Sep 7, 2017

The files that are causing problems are the ones named *.tests.info so can't we just stop the checker from checking those specific files? I'm going to attempt a PR, I'll see how hard that is.

@jenlampton jenlampton modified the milestones: 1.x-future, 1.7.3 Sep 7, 2017
@jenlampton jenlampton self-assigned this Sep 7, 2017
@jenlampton jenlampton removed this from the 1.7.3 milestone Sep 7, 2017
@jenlampton
Copy link
Member

I'm unable to reproduce this problem on backdrop 1.7.0 or above. Can someone confirm that it's still an issue?

@herbdool
Copy link

herbdool commented Sep 8, 2017

I can't reproduce either using honeypot.

@klonos
Copy link
Member Author

klonos commented Jun 29, 2018

This has now become an issue with #3105 which we are aiming to get in for the upcoming 1.11

@herbdool
Copy link

herbdool commented Jan 28, 2019

I added a PR to exclude .tests.info in the Updater class backdrop/backdrop#2491. The function was already smart enough to first try to fetch the .info that matches the directory name, and, if that didn't work, it would take the first .info file in the list. That seemed to exclude tests.info in my testing, but this PR will make sure.

I haven't been able to recreate the issue so perhaps someone else can test.

@quicksketch
Copy link
Member

That PR looks good to me @herbdool. I made a minor change to include the period in the substr() check, per @opi's comment that we shouldn't match on a string like footests.info.

@quicksketch
Copy link
Member

I've merged backdrop/backdrop#2491 into 1.x and 1.12.x. Thanks for the fix @herbdool and review from @opi!

@klonos
Copy link
Member Author

klonos commented May 27, 2019

Follow-up: #3821

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants