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

Fix usage of allowEmpty when also using include / exclude #106

Merged
merged 6 commits into from Feb 8, 2019
Merged

Fix usage of allowEmpty when also using include / exclude #106

merged 6 commits into from Feb 8, 2019

Conversation

chriskrycho
Copy link
Contributor

@chriskrycho chriskrycho commented Jan 23, 2019

Note that this implementation is a bit of a mess at this point. I'm happy to try to do some follow-on refactoring, but this solves the actual bug noted in #105 and identified while trying to fix adopted-ember-addons/ember-cli-sass#204.

Fixes #105.

index.js Outdated Show resolved Hide resolved
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

We need a test like

it('does not error with input node at a missing nested source', function() {
let inputPath = `${FIXTURE_INPUT}/dir1`;
let node = new Funnel(inputPath, {
srcDir: 'subdir3',
allowEmpty: true,
});
let expected = [];
builder = new broccoli.Builder(node);
return builder.build()
.then(results => {
let outputPath = results.directory;
expect(walkSync(outputPath)).to.eql(expected);
})
.then(() => builder.build())
.then(results => {
let outputPath = results.directory;
expect(walkSync(outputPath)).to.eql(expected);
});
});
});
(when allowEmpty is true, and input path doesn't exist). That test will likely answer your question RE: what to do in the fallback else case.

index.js Outdated Show resolved Hide resolved
index.js Show resolved Hide resolved
tests/index.js Outdated Show resolved Hide resolved
Avoid trolling anything downstream which may *expect* a folder to be
present, i.e. which does not understand or respect `allowEmpty` itself.
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

Sorry for the run around, could you also create a test like:

    it('creates nested output path when input node at a missing nested source', function() {
      let inputPath = `${FIXTURE_INPUT}/dir1`;
      let node = new Funnel(inputPath, {
        include: ['*'],
        srcDir: 'subdir3',
        destDir: 'some-place',
        allowEmpty: true,
      });

       let expected = ['some-place/'];

       builder = new broccoli.Builder(node);
      return builder.build()
        .then(results => {
          let outputPath = results.directory;

           expect(walkSync(outputPath)).to.eql(expected);
        })
        .then(() => builder.build())
        .then(results => {
          let outputPath = results.directory;

          expect(walkSync(outputPath)).to.eql(expected);

          return builder.build();
        })
        .then(results => {
          let outputPath = results.directory;

          expect(walkSync(outputPath)).to.eql(expected);
        });
    });

index.js Show resolved Hide resolved
@chriskrycho
Copy link
Contributor Author

Added that test also, @rwjblue.

@chriskrycho
Copy link
Contributor Author

Ping @rwjblue – AppVeyor failure is unrelated.

@rwjblue rwjblue merged commit 00de4f4 into broccolijs:master Feb 8, 2019
@rwjblue rwjblue added the bug label Feb 8, 2019
@rwjblue rwjblue changed the title Handle empty input nodes when include or exclude are set. Support allowEmpty when also using include / exclude Feb 8, 2019
@rwjblue rwjblue changed the title Support allowEmpty when also using include / exclude Fix usage of allowEmpty when also using include / exclude Feb 8, 2019
@rwjblue
Copy link
Member

rwjblue commented Feb 8, 2019

Released in v2.0.2.

@chriskrycho chriskrycho deleted the input-node-105 branch April 2, 2019 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants