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

Run tests with bin/test.sh #19

Merged
merged 4 commits into from Jun 21, 2020
Merged

Run tests with bin/test.sh #19

merged 4 commits into from Jun 21, 2020

Conversation

Morozzzko
Copy link
Contributor

@Morozzzko Morozzzko commented Jun 20, 2020

make test отваливалась из-за того, что нет test.sh

Кажется, оно особо и не нужно. Можно просто через бандл запустить
конкретные файлы тестов.

Проблема: вместо -Ilib приходится писать такую ужасную конструкцию как
-I "../../../lib". Оно фактически не проблема, но хотелось бы без
этого. Но кажется, что решение в целом норм

Проблема просто в том, что test.sh лежит в папке bin от корня проекта, а вызывали test.sh в директории упражнения. Подправил пути

@Morozzzko Morozzzko requested a review from mokevnin June 20, 2020 21:12
@mokevnin
Copy link
Contributor

Кажется, оно особо и не нужно. Можно просто через бандл запустить
конкретные файлы тестов.

Как показала практика очень даже нужно. Когда приходится делать какие то изменения, то это в ад превращается. Сейчас стандарт такой, везде кладется test.sh. Дальше просто добавляешь эту директорию в PATH и вуаля, вызов без всяких точек и без дублирования.

@Morozzzko
Copy link
Contributor Author

А, окей. Если в других проектах оказалось удобно, я посмотрю, как организовано, и перенесу.

@Morozzzko
Copy link
Contributor Author

А, всё было уже сделано. Обновлю описание PR

@Morozzzko Morozzzko changed the title Run tests with bundle instead of test.sh Run tests with bin/test.sh Jun 20, 2020
@@ -1,2 +1,2 @@
test:
@ test.sh
@ ../../../bin/test.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Давай только в PATH добавим чтобы путь не указывать. Потом мы в другие образа это добавим.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ща плохо соображаю. хочешь в главном мейкфайле PATH выставить? я не проворачивал эти манипуляции с путями

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ага, понял. done

Copy link
Contributor

Choose a reason for hiding this comment

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

В Dockerfile PATH а тут просто test.sh

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

@mokevnin mokevnin Jun 20, 2020

Choose a reason for hiding this comment

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

Оно должно быть в докере потому что все это добро будет запускаться в докере. Без общего Makefile. Он используется только в разработке.

Я гоняю без докера и в целом ок

Под это точно не подстраиваемся. У нас же универсальная система, а там кроме тестов еще куча всего запускается.

Copy link
Contributor Author

@Morozzzko Morozzzko Jun 20, 2020

Choose a reason for hiding this comment

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

Хм, а ведь в докере оно есть.

ENV PATH=/exercises-ruby/bin:$PATH

А вот в разработке оно не работает даже с ним. Завтра тогда поразбираюс, спасибо

@mokevnin mokevnin merged commit 3a079d0 into master Jun 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants