Vega-strike-engine-source: Define validation procedure for Pull Requests

Created on 12 Apr 2020  Â·  12Comments  Â·  Source: vegastrike/Vega-Strike-Engine-Source

I suggest that we define a validation procedure for Pull Requests, that will present a basic acceptance criteria for changes based on tests that the CI can't make.
I'm still new to the VS universe to say all the details, but I'd that it'd be something like:

  • Start with ship A in location XYZ. Fly to N.
  • Start with ship B in location ___. Make a fight.
  • Fly into a nebula.
  • Etc.

It should be a set of short tests that will test different aspects of the game, widely enough to make sure that a change hasn't broken anything drastic.
I would even say, maybe we can create a set of savefiles, to make the process easier.

If you think that doing that per PR is too much, we can still use that per release.

Most helpful comment

@nabaco we probably should give step by step instructions.

For instance:

  1. Launch
  2. Go to Max speed
  3. Reduce speed to zero
  4. Retro-burn and reduce velocity to zero
  5. Go to 150kps
  6. Turn left for 5 seconds
  7. Go straight for 5 seconds
  8. Turn right for 5 seconds
    etc.

We want to make sure people are doing the same thing.

I realize the dogfight might be harder to script; but let's do what we can. it'll help with repeatability and validating bugs that get filed. We can also then ask how the deviated from the script to get the bug (jumped to X instead of Y, used ship G instead of F, etc).

We can specify to use a new game for a of these. Well eventually want to add some saved games so we can test other things where one had to start from a different location, or use a different kind of ship.

Even after we get unit testing in these will still be helpful for verification even if not performed as often then.

All 12 comments

For now, I fly, fire guns and missiles. If I have more time, I pick a fight with Oswald.
Once the code is better decoupled, we can write unit tests.

Get Outlook for Androidhttps://aka.ms/ghei36


From: Nachum Barcohen notifications@github.com
Sent: Monday, April 13, 2020 12:17:59 AM
To: vegastrike/Vega-Strike-Engine-Source Vega-Strike-Engine-Source@noreply.github.com
Cc: Subscribed subscribed@noreply.github.com
Subject: [vegastrike/Vega-Strike-Engine-Source] Define validation procedure for Pull Requests (#68)

I suggest that we define a validation procedure for Pull Requests, that will present a basic acceptance criteria for changes based on tests that the CI can't make.
I'm still new to the VS universe to say all the details, but I'd that it'd be something like:

  • Start with ship A in location XYZ. Fly to N.
  • Start with ship B in location ___. Make a fight.
  • Fly into a nebula.
  • Etc.

It should be a set of a short tests that will test different aspects of the game, widely enough to make sure that a change hasn't broken anything drastic.
I would even say, maybe we can create a set of savefiles, to make the process easier.

If you think that doing that per PR is too much, we can still use that per release.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHubhttps://github.com/vegastrike/Vega-Strike-Engine-Source/issues/68, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACQWBEG7D4V7F2CG4ENWKZLRMIVYPANCNFSM4MGSANGQ.

That does sound like a good bare minimum :smile: I would add to that docking, a SPEC flight, and a jump, and that should get most things tested.

I'd add docking to a non-planet location (mining base, etc) as well.
Also, let's note that this has to be on a single monitor; multi-monitor doesn't work.

@nabaco can you please add a document either to the repo or Wiki outlining these steps and what do to.

No problem, will do

@BenjamenMeyer done: https://github.com/vegastrike/Vega-Strike-Engine-Source/wiki/Pull-Request-Validation

I'll be glad to hear your feedback. If it seems OK, we can close the issue for now.

@nabaco looks like a good start. We should get more detail in around each of the validation steps so folks can follow a planned script.

@Loki1950 might want to add some information there on a proper setup configuration (window mode) and some information about looking at the output from a terminal that it's running in while doing the steps.

@BenjamenMeyer What sort of extra detail are you thinking about? I'll be glad to add when I have some free time.

@nabaco we probably should give step by step instructions.

For instance:

  1. Launch
  2. Go to Max speed
  3. Reduce speed to zero
  4. Retro-burn and reduce velocity to zero
  5. Go to 150kps
  6. Turn left for 5 seconds
  7. Go straight for 5 seconds
  8. Turn right for 5 seconds
    etc.

We want to make sure people are doing the same thing.

I realize the dogfight might be harder to script; but let's do what we can. it'll help with repeatability and validating bugs that get filed. We can also then ask how the deviated from the script to get the bug (jumped to X instead of Y, used ship G instead of F, etc).

We can specify to use a new game for a of these. Well eventually want to add some saved games so we can test other things where one had to start from a different location, or use a different kind of ship.

Even after we get unit testing in these will still be helpful for verification even if not performed as often then.

The terminal output shows stdout and stderr streams therefore giving exactly where in the code something goes wrong I also would pipe the terminal output to 2 files so that the data persists and can be shared with other bug stampers

Looking good, you guys!

Let's make a PR template with some basics so folks can check off what has been done to validate PRs, at least until we have unit testing available.

129 and #130 satisfy this, and it can be closed once they're merged.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

royfalk picture royfalk  Â·  6Comments

nabaco picture nabaco  Â·  3Comments

stephengtuggy picture stephengtuggy  Â·  3Comments

stephengtuggy picture stephengtuggy  Â·  4Comments

LifWirser picture LifWirser  Â·  6Comments