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

feat(deno): allow test to succeed if there are no tests to run #305

Merged
merged 5 commits into from
Jun 28, 2023

Conversation

charsleysa
Copy link
Contributor

No description provided.

Copy link
Contributor

@barbados-clemens barbados-clemens left a comment

Choose a reason for hiding this comment

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

We should make this configurable. I'm okay with making it default to be on, like we do with @nx/jest, but people should be able to opt out if they so wish.

packages/deno/src/executors/test/test.impl.ts Outdated Show resolved Hide resolved
@charsleysa charsleysa changed the title fix(deno): allow test to succeed if there are no tests to run feat(deno): allow test to succeed if there are no tests to run Jun 26, 2023
@charsleysa
Copy link
Contributor Author

@barbados-clemens I've updated the PR according to your feedback.

I have added allowNone as an option. I have not made it default to true inside the executor in order to keep the functionality the same for existing projects. I have added the option to the generators so that new projects are generated with allowNone set to true.

Copy link
Contributor

@barbados-clemens barbados-clemens left a comment

Choose a reason for hiding this comment

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

I am okay with changing this as a default as it's more or less the behavior expected IMO.

You did set the default to be true, so go a head and remove setting in the project configuration and leave it true.

If you're worried about breaking people then you can add a migration to update that property for everyone to be 'false' so it doesn't change, but IMO it's not a big deal.

@nx-cloud
Copy link

nx-cloud bot commented Jun 27, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 36dc4af. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 4 targets

Sent with 💌 from NxCloud.

@charsleysa
Copy link
Contributor Author

@barbados-clemens thanks for the feedback! Looks like I misunderstood how the default field in the schema functioned. I've made the changes as recommended.

@barbados-clemens barbados-clemens merged commit d6b3b68 into nrwl:main Jun 28, 2023
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.

2 participants