fcsonline/drill

Tags do not apply to included files

michaelsproul opened this issue · 2 comments

I tried defining a top-level file to combine requests from multiple files like this:

# all.yaml
---
base: "http://localhost:3000"
plan:
  - include: ./file1.yaml
  - include: ./file2.yaml
# file1.yaml
---
- name: is_tagged
  request:
    url: /is_tagged
  tags: [example1]
# file1.yaml
---
- name: is_not_tagged
  request:
    url: /is_not_tagged

I found that tags in file1.yaml and file2.yaml are ignored. Running with --tags example1 doesn't include the is_tagged action from file1.yaml (it includes nothing).

I had a play with the code and it's because the tag system operates on the unexpanded YAML and expects tags on top-level items only, here:

drill/src/tags.rs

Lines 29 to 55 in f7627bf

pub fn should_skip_item(&self, item: &Yaml) -> bool {
match item["tags"].as_vec() {
Some(item_tags_raw) => {
let item_tags: HashSet<&str> = item_tags_raw.iter().map(|t| t.as_str().unwrap()).collect();
if let Some(s) = &self.skip_tags {
if !s.is_disjoint(&item_tags) {
return true;
}
}
if let Some(t) = &self.tags {
if item_tags.contains("never") && !t.contains("never") {
return true;
}
if !t.is_disjoint(&item_tags) {
return false;
}
}
if item_tags.contains("always") {
return false;
}
if item_tags.contains("never") {
return true;
}
self.tags.is_some()
}
None => self.tags.is_some(),
}

As a workaround I'm grouping my include files by their tags like this:

# all.yaml
plan:
  - include: ./file1.yaml
    tags: [example1]
  - include: ./file2.yaml

Figured I would document this here so that this behaviour is known. If we wanted to codify it maybe tags on included files could be rejected when loading/parsing? Alternatively, the tag system could be extended to work with recursively included files.

Related: #132

Thanks for this report! I vote for the recursive approach because it's the most intuitive one. I can work on a fix.

Not able to battle test this changes but I think it fixes the issue. It would be awesome to add some tests for it.
#167