20171026190443925__demo_data_from_9.0.1.sql

Clone Tools
  • last updated a few seconds ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
yes and I changed it

yes and I changed it

done

done

done

done

done

done

done

done

done

done

I like this approach much better; just made a few minor suggestions.

I like this approach much better; just made a few minor suggestions.

Shouldn't this reference the ORDERABLES constant?

Shouldn't this reference the ORDERABLES constant?

Rename this constant to something a bit more descriptive, like TABLE_ORDERABLES, since it is referencing the orderables table.

Rename this constant to something a bit more descriptive, like TABLE_ORDERABLES, since it is referencing the orderables table.

Rename this constant to something a bit more descriptive, like TABLE_ORDERABLES, since it is referencing the orderables table.

Rename this constant to something a bit more descriptive, like TABLE_ORDERABLES, since it is referencing the orderables table.

Replace this string literal with a descriptive constant, like INITIAL_TARGET.

Replace this string literal with a descriptive constant, like INITIAL_TARGET.

Can we rename this? It's confusing because it isn't getting a migration, but a Flyway target. Something like getTestMigrationTarget()?

Can we rename this? It's confusing because it isn't getting a migration, but a Flyway target. Something like getTestMigrationTarget()?

Can we rename this? It's confusing because it isn't getting a migration, but a Flyway target. Something like getTargetBeforeTestMigration()?

Can we rename this? It's confusing because it isn't getting a migration, but a Flyway target. Something like getTargetBeforeTestMigration()?

I will create a doc on wiki page when I close this review, present it on the tech call and when we all be on the same page, I will add page to docs.

I will create a doc on wiki page when I close this review, present it on the tech call and when we all be on the same page, I will add page to docs.

We should have some documentation how to write such tests, e.g. here http://docs.openlmis.org/en/latest/conventions/testing.html

We should have some documentation how to write such tests, e.g. here http://docs.openlmis.org/en/latest/conventions/testing.html

the migration has been removed so I will resolve this comment

the migration has been removed so I will resolve this comment

I modified tests and now we put data before test.

I modified tests and now we put data before test.

All demo data migrations have been removed

All demo data migrations have been removed

I changed test names to migration summary

I changed test names to migration summary

OLMIS-4236: Modified migration tests to stop using data from migrations

    • -28
    • +0
    ./20171026190443925__demo_data_from_9.0.1.sql
  1. … 11 more files in changeset.
I wanted avoid test names like 201802052111153916MigrationIntegrationTest. I will try to find a better names

I wanted avoid test names like 201802052111153916MigrationIntegrationTest. I will try to find a better names

This was the easiest way because if we will create a data before each test we will probably create data for only right path and we could omit some edge cases but I will try to modify tests to avoid...

This was the easiest way because if we will create a data before each test we will probably create data for only right path and we could omit some edge cases but I will try to modify tests to avoid those additional migrations.

In the v9.0.1 demo data we have situation where there are few same supervisorynode and program combo. This script simple remove old supply lines and insert correct to handle the constraint that is ...

In the v9.0.1 demo data we have situation where there are few same supervisorynode and program combo. This script simple remove old supply lines and insert correct to handle the constraint that is added in the next migration.

it's always one milisecond before/after migration

it's always one milisecond before/after migration

See my comment about not adding demo data to migrations. The timestamp for this migration file name doesn't seem to match the migration it's related to?

See my comment about not adding demo data to migrations. The timestamp for this migration file name doesn't seem to match the migration it's related to?

See my comment in the previous migration about not adding demo data to migrations. I read the email to the dev forum, but I don't quite understand the purpose of this migration script?

See my comment in the previous migration about not adding demo data to migrations. I read the email to the dev forum, but I don't quite understand the purpose of this migration script?

I'm not sure this is a good naming convention for these tests? What if we add another migration related to dispensables; what will we name that one?

I'm not sure this is a good naming convention for these tests? What if we add another migration related to dispensables; what will we name that one?

This doesn't make sense to me. We should not be adding demo data to our migrations. Implementations do not want this extraneous data in their systems. Sorry, I see that this is in the integration-...

This doesn't make sense to me. We should not be adding demo data to our migrations. Implementations do not want this extraneous data in their systems.

Sorry, I see that this is in the integration-test section. However, I don't like the approach of putting in an entire set of data in order to test a migration. We should only put in enough data to test the test cases.