Skip to content
This repository was archived by the owner on Mar 18, 2022. It is now read-only.

Add submodule configuration #47

Closed
wants to merge 5 commits into from
Closed

Add submodule configuration #47

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8527520
Add tests for submodules
stsewd Mar 22, 2018
85989fb
Add more tests
stsewd Mar 22, 2018
30d66e4
Fix tests
stsewd Mar 23, 2018
8115e5e
Validate submodules
stsewd Mar 23, 2018
6cffaeb
Allow ALL keyword on submodules.exclude key
stsewd Mar 31, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 187 additions & 0 deletions readthedocs_build/config/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,193 @@ def it_uses_validate_file(tmpdir):
assert args[0] == 'setup.py'


def describe_validate_submodules():

def it_defaults_to_all_submodules(tmpdir):
with tmpdir.as_cwd():
source_file = tmpdir.join('subdir', 'readthedocs.yml')
build = get_build_config({}, source_file=str(source_file))
build.validate_submodules()
# TODO: This will be the dafult behavior?
Copy link
Member Author

Choose a reason for hiding this comment

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

What I mean here is: on other parts of the code, when the key is not given, it doesn't get added to the build dict, and then the default values are chosen by rtd, I think with that current behavior we are repeating some logic.

assert 'submodules' in build
assert build['submodules'] == {
'include': [],
'recursive': False,
}

def it_parses_include_key(tmpdir):
apply_fs(tmpdir, {'sub-one': {}, 'sub-two': {}})
with tmpdir.as_cwd():
build = get_build_config({
'submodules': {
'include': ['sub-one/', 'sub-two'],
}
})
build.validate_submodules()
assert 'submodules' in build
assert 'exclude' not in build['submodules']
assert build['submodules']['include'] == ['sub-one/', 'sub-two']
assert build['submodules']['recursive'] is False

def it_validates_include_key(tmpdir):
apply_fs(tmpdir, {'sub-one': {}, 'sub-two': {}})
with tmpdir.as_cwd():
build = get_build_config({
'submodules': {
'include': ['sub-one/', 'sub-too'],
}
})
with raises(InvalidConfig) as excinfo:
build.validate_submodules()
assert excinfo.value.key == 'submodules.include'
assert excinfo.value.code == INVALID_PATH

def it_parses_exclude_key(tmpdir):
apply_fs(tmpdir, {'sub-one': {}, 'sub-two': {}})
with tmpdir.as_cwd():
build = get_build_config({
'submodules': {
'exclude': ['sub-two'],
}
})
build.validate_submodules()
assert 'submodules' in build
assert 'include' not in build['submodules']
assert build['submodules']['exclude'] == ['sub-two']
assert build['submodules']['recursive'] is False

def it_validates_exclude_key(tmpdir):
apply_fs(tmpdir, {'sub-one': {}, 'sub-two': {}})
with tmpdir.as_cwd():
build = get_build_config({
'submodules': {
'exclude': ['sub-too'],
}
})
with raises(InvalidConfig) as excinfo:
build.validate_submodules()
assert excinfo.value.key == 'submodules.exclude'
assert excinfo.value.code == INVALID_PATH

def it_does_not_allow_exclude_and_include(tmpdir):
build = get_build_config({
'submodules': {
'include': ['sub-one'],
'exclude': ['sub-two'],
}
})
with raises(InvalidConfig) as excinfo:
build.validate_submodules()
assert excinfo.value.key == 'submodules'
assert excinfo.value.code == INVALID_KEYS_COMBINATION

def it_requires_include_with_recursive(tmpdir):
build = get_build_config({
'submodules': {
'recursive': True,
}
})
with raises(InvalidConfig) as excinfo:
build.validate_submodules()
assert excinfo.value.key == 'submodules'
assert excinfo.value.code == MISSING_REQUIRED_KEY

def it_parses_recursive_key(tmpdir):
apply_fs(tmpdir, {'sub-one': {}, 'sub-two': {}})
with tmpdir.as_cwd():
build = get_build_config({
'submodules': {
'exclude': ['sub-two'],
},
'recursive': True,
})
build.validate_submodules()
assert build['submodules']['recursive'] is True

def it_validates_recursive_key(tmpdir):
apply_fs(tmpdir, {'sub-one': {}, 'sub-two': {}})
with tmpdir.as_cwd():
build = get_build_config({
'submodules': {
'exclude': ['sub-two'],
},
'recursive': 'True',
})
with raises(InvalidConfig) as excinfo:
build.validate_submodules()
assert excinfo.value.key == 'submodules.recursive'
assert excinfo.value.code == INVALID_BOOL

def it_accepts_all_keyword_on_include(tmpdir):
apply_fs(tmpdir, {'sub-one': {}, 'sub-two': {}})
with tmpdir.as_cwd():
build = get_build_config({
'submodules': {
'include': 'all',
},
'recursive': True
})
build.validate_submodules()
assert build['submodules']['include'] == ALL
assert build['submodules']['recursive'] is True

def it_validates_the_correct_type_for_include(tmpdir):
apply_fs(tmpdir, {'sub-one': {}, 'sub-two': {}})
with tmpdir.as_cwd():
build = get_build_config({
'submodules': {
'include': 'none',
},
'recursive': True
})
with raises(InvalidConfig) as excinfo:
build.validate_submodules()
assert excinfo.value.key == 'submodules.include'
assert excinfo.value.code == INVALID_LIST

def it_validates_the_correct_type_for_exclude(tmpdir):
apply_fs(tmpdir, {'sub-one': {}, 'sub-two': {}})
with tmpdir.as_cwd():
build = get_build_config({
'submodules': {
'exclude': 'all',
Copy link
Member

Choose a reason for hiding this comment

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

If when submodules key is not present we are defaulting to include all of them, we need to support exclude: all --otherwise, how we say that we don't want to checkout submodules>

Copy link
Member Author

Choose a reason for hiding this comment

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

We do it with include: []

Copy link
Member Author

Choose a reason for hiding this comment

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

Still would be necessary exclude: all? I think it's more clear with the other form only.

Copy link
Member

Choose a reason for hiding this comment

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

To me, include: [] doesn't have a good meaning and it's confusing. It's more explicit to say exclude: all from my understanding.

},
'recursive': True
})
with raises(InvalidConfig) as excinfo:
build.validate_submodules()
assert excinfo.value.key == 'submodules.exclude'
assert excinfo.value.code == INVALID_LIST

def it_parses_explicit_defaults(tmpdir):
apply_fs(tmpdir, {'sub-one': {}, 'sub-two': {}})
with tmpdir.as_cwd():
build = get_build_config({
'submodules': {
'include': [],
'recursive': False,
}
})
build.validate_submodules()
assert 'submodules' in build
assert build['submodules']['include'] == []
assert build['submodules']['recursive'] is False

def it_parses_explicit_wrong_defaults(tmpdir):
Copy link
Member Author

Choose a reason for hiding this comment

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

What would be the behavior for this case? Should be an error? Ignore this? or change the value of recursive to False (this is implemented on this test case)?

apply_fs(tmpdir, {'sub-one': {}, 'sub-two': {}})
with tmpdir.as_cwd():
build = get_build_config({
'submodules': {
'include': [],
'recursive': True,
}
})
build.validate_submodules()
assert 'submodules' in build
assert build['submodules']['include'] == []
assert build['submodules']['recursive'] is False


def test_valid_build_config():
build = BuildConfig(env_config,
minimal_config,
Expand Down