Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

Add end-to-end tests for command-line interface #199

Merged
merged 16 commits into from
Oct 6, 2020
Merged

Add end-to-end tests for command-line interface #199

merged 16 commits into from
Oct 6, 2020

Conversation

MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented Oct 4, 2020 •
edited
Loading

Verifies that generated all.css contains the :root string on CI. It doesn't do any actual CSS parsing to verify its full validity, only does a simple grep check, but I hope this will be enough to prevent issues like #198 in the future.

@MaxDesiatov MaxDesiatov added the continuous integration Changes to the continuous integration process label Oct 4, 2020
@MaxDesiatov MaxDesiatov marked this pull request as ready for review October 4, 2020 15:47
@MaxDesiatov MaxDesiatov requested a review from mattt October 4, 2020 15:47
@mattt
Copy link
Contributor

mattt commented Oct 5, 2020

Thanks for getting started on this, @MaxDesiatov. I agree with your instinct to start with a shell script for this. Given that the primary offering of swift-doc is the CLI, do you think that it'd make sense to invest in a formal Swift test target that exercises this and other behavior of the executable?

@MaxDesiatov
Copy link
Contributor Author

Sure, let's do that. I'll update the PR accordingly 👍

@MaxDesiatov
Copy link
Contributor Author

@mattt ready for review 🙂

@mattt
Copy link
Contributor

mattt commented Oct 6, 2020

@MaxDesiatov Nice! Taking a look now.

mattt
mattt suggested changes Oct 6, 2020
View reviewed changes
Copy link
Contributor

@mattt mattt left a comment

Choose a reason for hiding this comment

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

A few suggested changes for your consideration. Would it be alright if I iterate on this some more this morning, and add some more test cases (e.g. to handle CSS and HTML formats)?

@MaxDesiatov @mattt
Apply suggestions from code review
07c2906 
Co-authored-by: Mattt <mattt@me.com>
@MaxDesiatov
Copy link
Contributor Author

Sure, please do!

@mattt mattt self-requested a review October 6, 2020 16:41
MaxDesiatov
MaxDesiatov commented Oct 6, 2020
View reviewed changes
@mattt @MaxDesiatov
Update Changelog.md
62d02ef 
Co-authored-by: Max Desiatov <max@desiatov.com>
@mattt
Copy link
Contributor

mattt commented Oct 6, 2020

@MaxDesiatov Any last changes before this gets merged?

@MaxDesiatov
Copy link
Contributor Author

LGTM, thank you for finishing this up!

@MaxDesiatov
Copy link
Contributor Author

Can I merge this now? Do you prefer merge commits or linear history by the way?

@mattt
Copy link
Contributor

mattt commented Oct 6, 2020

@MaxDesiatov I like to squash and merge PRs. Feel free to do the honors of hitting that green button 😄 (and thanks again for your help with this!)

@MaxDesiatov MaxDesiatov merged commit 006dddc into master Oct 6, 2020
@mattt mattt changed the title Test that all.css contains a valid CSS selector Add end-to-end tests for command-line interface Oct 6, 2020
@MaxDesiatov MaxDesiatov deleted the maxd/test-css branch October 6, 2020 17:14
@mattt mattt mentioned this pull request Oct 7, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mattt mattt mattt approved these changes

Assignees
No one assigned
Labels
continuous integration Changes to the continuous integration process
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MaxDesiatov @mattt