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

CLI Tests: Deploy all Python Content Types #816

Closed
wants to merge 21 commits into from

Conversation

kgartland-rstudio
Copy link
Collaborator

@kgartland-rstudio kgartland-rstudio commented Jan 16, 2024

Intent

Type of Change

    • Bug Fix
    • New Feature
    • Breaking Change
    • Documentation
    • Refactor
    • Tooling

Approach

This PR deploys all content. All Connect Jumpstart examples are added as well as various supported APIs. It also introduces a CONTENT variable where you can specify a single piece of content rather than deploying all.

After deploying, we check the Content Type using the API. I've added a .env file to each content folder to specify the expected content type to compare against.

This PR also disables the nightly Cypress tests until we're ready to re-enable those (they're currently failing).

Automated Tests

These are them.

Directions for Reviewers

The deployments will only run during the Nightly tests, these are passing here:

https://github.com/posit-dev/publisher/actions/runs/7544245350/job/20537305258

This PR has 91 files but most of those are the added contents, feel free to ignore anything in the test/sample-content/python folder.

Checklist

Copy link
Collaborator

@dotNomad dotNomad left a comment

Choose a reason for hiding this comment

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

I did not fully review this but:

  • looks like you have some duplicate commits in here perhaps something got a bit messed up in Git
  • we are adding some large files including images in this PR, should we consider separating this out to avoid bloating the size of this repo?

@kgartland-rstudio
Copy link
Collaborator Author

  • we are adding some large files including images in this PR, should we consider separating this out to avoid bloating the size of this repo?

The sample content was pulled in from the Jumpstart examples from Connect. I removed the images and README files, good catch.

@kgartland-rstudio
Copy link
Collaborator Author

  • looks like you have some duplicate commits in here perhaps something got a bit messed up in Git

I think I had to pull in changes from main at one point, is that what you mean? Other than that, I did have some trouble getting the tests to work in CI with Windows so I may have made the same commit comment multiple times.

@dotNomad
Copy link
Collaborator

I think I had to pull in changes from main at one point, is that what you mean? Other than that, I did have some trouble getting the tests to work in CI with Windows so I may have made the same commit comment multiple times.

I'm looking at the commits in the PR you have commits you have written, but also some from me and Taylor. Like the HTML5 history one has a different SHA than the commit on main (f1cdb744f9a5399a071aaaa12d5d04214dd7b776 vs 028dd33643682c7ff580ba1715c470a2ebfa1bc6). This tells me something incorrectly was done with a merge, cherry pick, or where branching occurs. I can definitely help clean this up if you'd like to avoid the duplication of commits on merge (due to the duplicate SHAs)

@dotNomad
Copy link
Collaborator

The sample content was pulled in from the Jumpstart examples from Connect. I removed the images and README files, good catch.

Thanks for removing these. I never like to include non-code like images in a repo if it can be helped. I've worked with too many multiple GBs repos in the past that felt so sluggish due to this.

@tdstein
Copy link
Collaborator

tdstein commented Jan 18, 2024

The sample content was pulled in from the Jumpstart examples from Connect. I removed the images and README files, good catch.

Thanks for removing these. I never like to include non-code like images in a repo if it can be helped. I've worked with too many multiple GBs repos in the past that felt so sluggish due to this.

Should we have a separate repository for sample content? Is there an advantage to reusing the sample content across this project and Connect?

Also, if necessary, we can place large non-text files in S3.

@mmarchetti
Copy link
Contributor

It would be good to squash this PR when merging so the images won't be included (a regular merge will preserve the commit that added them and the commit that removed them).

@kgartland-rstudio
Copy link
Collaborator Author

kgartland-rstudio commented Jan 18, 2024

Since we have all of this content in Connect, I could do a sparse-checkout from Connect and get the examples folder where the Jumpstart examples are stored.

Also for context, I'm using the Jumpstart examples because we publish those in the Connect product as examples for customers so we'll want to make sure those work with the publisher.

@dotNomad
Copy link
Collaborator

It would be good to squash this PR when merging so the images won't be included (a regular merge will preserve the commit that added them and the commit that removed them).

Since we have all of this content in Connect, I could do a sparse-checkout from Connect and get the examples folder where the Jumpstart examples are stored.

Perhaps it would be best to close up this PR, and re-branch for the new work if we will be grabbing the sample content from Connect.

@kgartland-rstudio kgartland-rstudio mentioned this pull request Jan 31, 2024
7 tasks
@mmarchetti
Copy link
Contributor

Can this be closed now that #926 is open?

@kgartland-rstudio
Copy link
Collaborator Author

Can this be closed now that #926 is open?

Yup! Closing...

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.

4 participants