change importing in all tests

vadim9999 commented 5 years ago

Now in all tests we have this structure that get object.

const {allergiesFilePath} = require('../../../files.js')
const allergies = require(allergiesFilePath)

Replace it with importing from filesObjects.js that is in root of project like this

const staticData = require("./filesObjects")
// to get acces use staticData.allergies

vadim9999 commented 5 years ago

@EbrahimKreem

atherdon commented 5 years ago

i think confusion between having both es5 & es6 versions inside one package are a real sign that we need to split code into few repositories and make it less confusing for developers...

vadim9999 commented 5 years ago

Rollup works with es6, mdoule is es5. Should we change rollup on es5?

atherdon commented 5 years ago

this is not what i mean. major part of sd repo(all files inside src) are actually working with es6. and only small piece of code using es5. and i think it's just another "+" of making code less complicated by separation of code again.

Don't worry - if i'm right - life will force us soon to do it :) Especially with an increased number of new interns. So let's see in 1-2 weeks and then maybe we'll make a decision together.

Or if you and Ebrahim have time and ready - we might start to work on separation when you ready to do it. I just cleaning my inbox last 2 days and not ready to jump on board and code

vadim9999 commented 5 years ago

But src contains with static data and tests. tests should be in es5 or es6? If we wil use es6 need uncoment fix in babel.conf that I have added.

atherdon commented 5 years ago

right now src contains json(static data), tests and export. but actually, only export will be changed frequently. So with work of rollup-copy-asset-module we'll be able to separate things. I'll add more details a bit later and share with all team members at main task.

"tests" are actually not an easy term. because we have json-files-tests, jsonlint-tests and some other. So i think for tests we can talk about 3 different instances. json-files-tests i think use es6, jsonlint-tests use es5, and other tests also use es6. I may be wrong, but this is how i imagined it.

vadim9999 commented 5 years ago

Am I right understood tests in src/data should be in es6?

atherdon commented 5 years ago

give me a code link as example please

vadim9999 commented 5 years ago

https://github.com/GroceriStar/sd/blob/master/src/data/__tests__/allergy.test.js

vadim9999 commented 5 years ago

@atherdon What we will do with version es5 or es6 in tests in src/data?

atherdon commented 5 years ago

https://github.com/GroceriStar/sd/blob/master/src/data/__tests__/allergy.test.

just checked history and this file at sd-2.0.3 has this line:

import allergies from '~/Allergy/allergies.json' is it was necessary to change to es5?

vadim9999 commented 5 years ago

No it was worked good. But you have created task about replacing importing of all tests with path resolver

ebrahim-2 commented 5 years ago

😅 should I make a patch in src/data/tests for using es5 or just keep it with es6?

atherdon commented 5 years ago

No it was worked good. But you have created task about replacing importing of all tests with path resolver

I think 80% probability is I forget or have a mistake :) because i have a memory of fish.

@EbrahimKreem not sure to be honest. this problem is more than it looks. these "tests" problem confusing a lot of people. And yesterday our team join a Q&A engineer(tester). And i'm sure that she will be confused about our 3 type of tests as well.

As you join a conversation - we can discuss my other idea, while Vadim will sleep.

I was thinking about changing a project structure a bit. I'm conflicted, but it definitely will simplify work for Q&A people.

So, instead of having

https://github.com/GroceriStar/sd/tree/master/src/data/__tests__
https://github.com/GroceriStar/sd/tree/master/generator (yeah, we should have tests here too)
https://github.com/GroceriStar/sd/blob/master/jsonlint.config.js
https://github.com/GroceriStar/sd/blob/master/src/index.test.js

we'll have a one /test in root and a few sub-folders for keeping these tests separated.

inspired by: https://github.com/kriasoft/Folder-Structure-Conventions

ebrahim-2 commented 5 years ago

Yeah I agree with you the project structure is very confusing

ebrahim-2 commented 5 years ago

so you say that we should make a /test folder in the root dir and start moving files

atherdon commented 5 years ago

yeah, i'm thinking about it, but not sure if need to do it(right now). what do you think?

ebrahim-2 commented 5 years ago

"Simplicity is the soul of efficiency" - Austin Freeman if this step will make the structure more reasonable I think we should make it

atherdon commented 5 years ago

this structure will not last forever. it may be only for short term goals. as you may understand - when we'll move files to a new destination - a lot of our functionality will be broken, right? Do you think you want to do it?

ebrahim-2 commented 5 years ago

I'm after you 😆

ebrahim-2 commented 5 years ago

a lot of our functionality will be broken our hero is here

vscode

atherdon commented 5 years ago

lol :) in order to simplify it:

when we'll move https://github.com/GroceriStar/sd/tree/master/src/data/__tests__ we will need to update paths to json files
for https://github.com/GroceriStar/sd/blob/master/jsonlint.config.js we'll need to move files.js as well and update paths at package.json for calling jsonlint script
for https://github.com/GroceriStar/sd/blob/master/src/index.test.js i think we need to rename it, because it wouldn't be a very obvious name
not sure if we moved some tests, related to methods that we moved from fetch

and folder structure can be like

- test
-- static (for non generated json)
-- generator
-- generated-data(don't like this name but it's for keeping tests for generated json files)
-- jsonlint
-- json-schema(it's our other tests, for comparing structures. task is #22)

ok, i think i mentioned major problems - if you want you can grab this task, but maybe for making a separated PR from #269. i'll advise you to create a separated branch from master and did it. because other developers can work on this files and maybe we'll have some merge conflicts

atherdon commented 5 years ago

Ebrahim, as Vadim merged your latest changes - please work on this one then. Because right now all these test situations look crazier for me :)

ebrahim-2 commented 5 years ago

ok

vadim9999 commented 5 years ago

is it done?

atherdon commented 5 years ago

i'm not sure. our tests are mess. we made some work, but there still a lot of things to work on. This milestone contain all tasks related to tests. maybe it will help you: https://github.com/GroceriStar/sd/milestone/1

Food-Static-Data / sd

change importing in all tests #278