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

Libraries UI tests #35337

Merged
merged 14 commits into from Jul 8, 2020
Merged

Libraries UI tests #35337

merged 14 commits into from Jul 8, 2020

Conversation

maddiedierker
Copy link
Contributor

@maddiedierker maddiedierker commented Jun 16, 2020

Adds UI test coverage for our new libraries feature. Also adds a copy of the environment configuration and S3 bucket policy for using cdo-v3-libraries in CI environments.

Links

Reviewer Checklist:

  • Tests provide adequate coverage
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

Madelyn Kasula and others added 2 commits June 18, 2020 15:28
Co-authored-by: suresh <suresh@code.org>
@maddiedierker
Copy link
Contributor Author

@sureshc @uponthesun I fixed the new bucket policy in s3_buckets.yml (there were just a few syntax errors) and the changeset succeeded in CloudFormation. could you take a look to make sure the policy itself looks okay?

Copy link

@uponthesun uponthesun left a comment

Choose a reason for hiding this comment

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

I only looked at the S3 policy and ui_tests.sh changes, but those look good to me, though it seems like the test isn't passing on Drone yet? It could be useful to manually edit the bucket policy while iterating on the UI tests via sshing to drone container, and then put it in the template afterwards.

Feel free to ping me on slack if you want to brainstorm on how best to do this.

@maddiedierker
Copy link
Contributor Author

we don't add new S3 buckets + UI tests often, but adding the steps needed to do so here in case it comes up again:

  • Add a bucket policy to the relevant bucket in S3. This can usually be copied from another bucket. (I copied this one from cdo-v3-sources since they have similar purposes.)
  • Add that bucket policy to s3_buckets.yml so that it's stored in our codebase as well, not just in S3. (You'll need to convert the JSON in S3's UI to YAML, but there are other examples in s3_buckets.yml and throughout the codebase.)
  • Update the DroneWorker IAM policy to also allow S3 requests to the relevant bucket. This has to be done from the codeorg-dev AWS account, not your personal account. This can also be copied from another bucket -- we followed the example of cdo-v3-sources again.
  • Make sure your UI test is trying to access the correct directory within the S3 bucket. We have CI mapped to RAILS_ENV=test, so our code defaulted to looking at the libraries_test/ directory, and we had to specify the directory in ui_tests.sh.

thank you, winter and suresh, for all your help on this!

# Confirm Student1's library is in Student2's project
And I wait for the page to fully load
Then I open the Manage Libraries dialog
And I wait until element "a:contains('UntitledProject')" is visible
Copy link
Contributor

Choose a reason for hiding this comment

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

Might not be possible, but should we also check if the library blocks are in the project?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i started down that path, and it seemed a lot more complicated than it's worth


# Student2 imports Student1's library
Given I create a student named "Student2"
And I start a new Applab project
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you be able to add a test for Gamelab too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i definitely can, but it looks like libraries aren't enabled in Gamelab, so i'm not sure how much value that would add. since you're OOO this week, i'm going to leave as-is for now and we can decide if we want to refactor this when you're back

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops. I think we forgot to enable libraries in Gamelab. Shouldn't block this PR, but probably should happen at some point.

@maddiedierker
Copy link
Contributor Author

@jmkulwik PTAL!

@maddiedierker maddiedierker merged commit d671161 into staging Jul 8, 2020
@maddiedierker maddiedierker deleted the libs-ui-test branch July 8, 2020 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants