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

[REG-1579] add initial Automated Testing package docs #55

Merged
merged 6 commits into from
Jan 25, 2024

Conversation

analogrelay
Copy link
Contributor

@analogrelay analogrelay commented Jan 18, 2024

This is an initial draft of the "one-pager" Getting Started doc for the automated testing package (I think "Monkey Testing" is a little too obscure a name for this, so I was focusing on "Automated Testing" as an alternate).

TODO:

  • Merge a PR to RegressionGames.Unity.Testing with a few small UI improvements I made while walking through this process (REG-1587) - This doesn't have to be done before we merge this, but the docs will refer to things I've already fixed in my branch :). [REG-1587] Several tidy-ups before release RegressionGames.Unity.Testing#6
  • Add screenshots
  • Add GIF recording of Monkey bot
  • Release a v0.0.1 tag of the RegressionGames.Unity.Testing package on public GitHub. - This also doesn't have to be done before merge but the docs will refer to it as if it exists.

Find the pull request instructions here

Every reviewer and the owner of the PR should consider these points in their request (feel free to copy this checklist so you can fill it out yourself in the overall PR comment)

  • The code is extensible and backward compatible
  • New public interfaces are extensible and open to backward compatibility in the future
  • If preparing to remove a field in the future (i.e. this PR removes an argument), the argument stays but is no longer functional, and attaches a deprecation warning. A linear task is also created to track this deletion task.
  • Non-critical or potentially modifiable arguments are optional
  • Breaking changes and the approach to handling them have been verified with the team (in the Linear task, design doc, or PR itself)
  • The code is easy to read
  • Unit tests are added for expected and edge cases
  • Integration tests are added for expected and edge cases
  • Functions and classes are documented
  • Migrations for both up and down operations are completed
  • A documentation PR is created and being reviewed for anything in this PR that requires knowledge to use
  • Implications on other dependent code (i.e. sample games and sample bots) is considered, mentioned, and properly handled
  • Style changes and other non-blocking changes are marked as non-blocking from reviewers

Select **Add package from git URL** and paste in the following URL:

```
https://github.com/Regression-Games/RegressionGames.Unity.Testing.git?path=src/gg.regression.unity.testing#v0.0.1
Copy link
Contributor Author

@analogrelay analogrelay Jan 22, 2024

Choose a reason for hiding this comment

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

For testing the doc, I currently recommend using this Git URL instead:

REMOVED (see below)

When Regression-Games/RegressionGames.Unity.Testing#6 lands, you could also use:

https://github.com/Regression-Games/RegressionGames.Unity.Testing.git?path=src/gg.regression.unity.testing

To get the latest main version

Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be the SSH link, [email protected]:Regression-Games/RegressionGames.Unity.Testing.git, rather than the https link. I found out (through my own config) that if the user only ever uses SSH (as is the case with my computer), I have not http config for git, and even for public repos, it will fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohhh, I keep forgetting I have this in my global git config 🤣

[url "[email protected]:"]
	insteadOf = https://github.com

(It forces any https://github.com to become [email protected]:, so HTTPS URLs automatically use SSH instead)

Good call, I'll post an updated URL soon.

The doc is still fine though, since when the package goes live, the HTTPS URL will work because the repo will be public.

Copy link
Contributor Author

@analogrelay analogrelay Jan 23, 2024

Choose a reason for hiding this comment

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

After resolving some issues @abeizer found, please use this URL instead:

[email protected]:Regression-Games/RegressionGames.Unity.Testing.git?path=src/gg.regression.unity.testing#ashley/reg-1596

This is necessary until Regression-Games/RegressionGames.Unity.Testing#8 lands

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 have not http config for git, and even for public repos, it will fail.

We can provide the SSH URL too, but currently we only provide the HTTPS link in our docs for the existing SDK

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh my apologies! I confused myself and it was actually the reverse I experienced, duh... we need to use the https one and not the ssh one, THAT was the issue I originally had. So yes we can just keep the HTTPS one

docs/automated-testing/getting-started.md Outdated Show resolved Hide resolved
docs/automated-testing/getting-started.md Outdated Show resolved Hide resolved
Copy link
Contributor

@vontell vontell left a comment

Choose a reason for hiding this comment

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

Just did an initial read through, I need to actually go and complete the tutorial now. Will provide another update once I do that!

sidebar_label: Getting Started
---

# Getting Started with Automated Testing
Copy link
Contributor

Choose a reason for hiding this comment

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

Since right now this only encapsulate the UI-related components, I was wondering if we might call it "Automated UI Testing"? We can always change it, but I don't want it to get confused with some of our other docs and site that also claim automated testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍🏻

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this was changed yet?

docs/automated-testing/getting-started.md Outdated Show resolved Hide resolved
docs/automated-testing/getting-started.md Outdated Show resolved Hide resolved
Copy link
Contributor

@vontell vontell left a comment

Choose a reason for hiding this comment

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

Nice!! Was so easy to get this up and running. Like how quick it is to get started.

Mostly comments about clarifying certain points, and just a few small issues I ran into. Also, I notice that the logo is still not working on the overlay?

docs/automated-testing/getting-started.md Outdated Show resolved Hide resolved
Select **Add package from git URL** and paste in the following URL:

```
https://github.com/Regression-Games/RegressionGames.Unity.Testing.git?path=src/gg.regression.unity.testing#v0.0.1
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be the SSH link, [email protected]:Regression-Games/RegressionGames.Unity.Testing.git, rather than the https link. I found out (through my own config) that if the user only ever uses SSH (as is the case with my computer), I have not http config for git, and even for public repos, it will fail.

and Entity Discoverers (see below) should be added as children of the Automation Controller.
The Controller also provides APIs for spawning bots and managing recordings (via a separate, Automation Recorder component attached to the same object).

To add the Automation Controller to your scene, click **GameObject > Regression Games > Automation Controller** in the Unity menu.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the Unity menu? I went to the Unity menu in the toolbar. Might be good to clarify right clicking on the scene object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I meant the main menu bar, but right clicking also provides the same menu. I'll clarify!

In order to automate entities in your game, the Automation Controller needs to be able to find them.
The Automation Controller uses Entity Discoverers to find entities in the scene that can be automated.
The package provides a few built-in Entity Discoverers, but you can also create your own.
To add one of the built-in discoverers, open the **GameObject > Regression Games > Discovery** menu and select the discoverer you want to add:
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, also be sure to add that you should do this on the automation controller, unless it can be done at the top level as well (but we should at least tell them somewhere to click to get that menu)

Copy link
Contributor

Choose a reason for hiding this comment

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

I see... this comes after the picture. I'd put it before the picture so that we avoid the scenario I just had where I didn't scroll further to see more and got stuck on that

Copy link
Contributor

Choose a reason for hiding this comment

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

This "where to right click to get the menu" applies to further parts in these docs, so I'll avoid making that comment again and leave this as the last one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here, also be sure to add that you should do this on the automation controller, unless it can be done at the top level as well (but we should at least tell them somewhere to click to get that menu)

If you click that menu item without a node selected, it automatically targets the automation controller. Otherwise, it should give you a fairly clear error telling you the problem.

I see... this comes after the picture. I'd put it before the picture so that we avoid the scenario I just had where I didn't scroll further to see more and got stuck on that

I didn't want to put it the image above since the text that describes the image should generally precede it. I can do that if you think it's clearer though.

docs/automated-testing/getting-started.md Outdated Show resolved Hide resolved

![The Automation Controller component with the "Dont Destroy On Load" checkbox checked](img/getting-started/ac-dont-destroy-on-load.png)

## Running the Monkey Bot
Copy link
Contributor

Choose a reason for hiding this comment

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

Before this or at the beginning, make sure to mention that you should save the scene

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

@analogrelay
Copy link
Contributor Author

Also, I notice that the logo is still not working on the overlay?

Fixed if you use the ashley/reg-1596 branch (see Regression-Games/RegressionGames.Unity.Testing#8)

Copy link
Collaborator

@abeizer abeizer left a comment

Choose a reason for hiding this comment

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

It works! Things I really like about this:

  • Super easy to understand and get the bot running
  • Grouping concepts/functionality within + under the Automation Controller. If I know where my controller is, I know how to access important settings and data like discoverers and recordings.
  • Only capturing state + screenshot when it's relevant (not blowing up the .zip size)

One thing I don't see mentioned here is the "Destroy" option after you stop a bot -> It looks like this is supposed to be an easy way to delete recordings? It doesn't seem to be deleting my .zip files though (bug?) :-( I think its usefulness is also limited right now because if I stop my game and restart it, I don't see any previous bots in the overlay. This could be nice for a future task?

I have a mix of blocking and non-blocking comments.

docs/automated-testing/getting-started.md Outdated Show resolved Hide resolved
docs/automated-testing/getting-started.md Outdated Show resolved Hide resolved
docs/automated-testing/getting-started.md Outdated Show resolved Hide resolved
docs/automated-testing/getting-started.md Outdated Show resolved Hide resolved
docs/automated-testing/getting-started.md Outdated Show resolved Hide resolved
docs/automated-testing/getting-started.md Outdated Show resolved Hide resolved
docs/automated-testing/getting-started.md Outdated Show resolved Hide resolved
analogrelay and others added 2 commits January 24, 2024 11:14
Co-authored-by: Abby Beizer <[email protected]>
Co-authored-by: Aaron Vontell <[email protected]>
@analogrelay
Copy link
Contributor Author

PR Feedback addressed! Taking this out of draft as the remaining work is to record a GIF of the bot and I don't want to block people reviewing the doc on just that. I'll work on that right after lunch though.

@analogrelay analogrelay marked this pull request as ready for review January 24, 2024 19:47
@analogrelay
Copy link
Contributor Author

And now the GIF recording is in place!

Copy link
Contributor

@vontell vontell left a comment

Choose a reason for hiding this comment

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

Just cleaning up the addressed comments, I believe there are some that are still waiting to be addressed?

@analogrelay
Copy link
Contributor Author

Just cleaning up the addressed comments, I believe there are some that are still waiting to be addressed?

I thought I got them all, but feel free to let me know if I missed something!

@analogrelay analogrelay requested a review from vontell January 24, 2024 22:44
Copy link
Contributor

@addisonbgross addisonbgross left a comment

Choose a reason for hiding this comment

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

This reads really well, nice one!

@analogrelay analogrelay merged commit 65e30a5 into main Jan 25, 2024
1 check passed
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