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

[BUGFIX lts] blueprints: fix framework detection to work with prerelease versions of ember-cli-mocha #15935

Merged
merged 1 commit into from Dec 6, 2017

Conversation

simonihmig
Copy link
Contributor

semver.satisfies does not cover prerelease versions like *.*.*-beta as required, so use gte

cc @Turbo87

@Turbo87
Copy link
Member

Turbo87 commented Dec 6, 2017

@rwjblue I think we can make this [BUGFIX lts] too, right?

@simonihmig
Copy link
Contributor Author

BTW, it just came to my mind that these last two changes (support ember-mocha, ember-qunit and beta versions) here are currently not covered by tests. I can maybe add some in a separate PR?

But just wondered how. I don't think it makes sense to add them for each and every blueprint test case, making the number of permutations explode. But maybe just something like a smoke test, by adding those cases to one blueprint, e.g. component-test?

@rwjblue
Copy link
Member

rwjblue commented Dec 6, 2017

@rwjblue I think we can make this [BUGFIX lts] too, right?

Ya, I think so

@simonihmig simonihmig changed the title [BUGFIX beta] blueprints: fix framework detection to work with prerelease versions of ember-cli-mocha [BUGFIX lts] blueprints: fix framework detection to work with prerelease versions of ember-cli-mocha Dec 6, 2017
@simonihmig
Copy link
Contributor Author

done! [lts]

@rwjblue
Copy link
Member

rwjblue commented Dec 6, 2017

BTW, it just came to my mind that these last two changes (support ember-mocha, ember-qunit and beta versions) here are currently not covered by tests. I can maybe add some in a separate PR?

But just wondered how.

I would probably split things up and unit test the specific logic.

Something like:

function detectType(dependencies, root) {
  if ('ember-qunit' in dependencies || 'ember-cli-qunit' in dependencies) {
    type = 'qunit';

  } else if ('ember-mocha' in dependencies) {
    type = 'mocha-0.12';

  } else if ('ember-cli-mocha' in dependencies) {
    var checker = new VersionChecker({ root });
    if (fs.existsSync(this.path + '/mocha-0.12-files') && checker.for('ember-cli-mocha', 'npm').gte('0.12.0')) {
      type = 'mocha-0.12';
    } else {
      type = 'mocha';
    }

  } else {
    this.ui.writeLine('Couldn\'t determine test style - using QUnit');
    type = 'qunit';
  }

  return type;
}

module.exports = function(blueprint) {
blueprint.supportsAddon = function() {
    return false;
  };

  blueprint.filesPath = function() {
    var dependencies = this.project.dependencies();
    var type = detectType(dependencies, this.project.root);

    return path.join(this.path, type + '-files');
  };

  return blueprint;
};

module.exports._detectType = detectType;

This way you can pass any number of different permutations along with something like broccoli-test-helper (like I test ember-cli-version-checker itself)...

…ase versions of ember-cli-mocha

`semver.satisfies` does not cover prerelease versions like `*.*.*-beta` as required, so use `gte`
@rwjblue
Copy link
Member

rwjblue commented Dec 6, 2017

Sorry for the rebase issues @simonihmig, thanks for the quick turn around.

@rwjblue rwjblue merged commit 6115234 into emberjs:master Dec 6, 2017
@simonihmig simonihmig deleted the framework-detection branch December 7, 2017 23:05
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.

None yet

3 participants